[Qemu-devel] [RFC PATCH 0/8] Add Generic PCI host device update

[Qemu-devel] [RFC PATCH 0/8] Add Generic PCI host device update

[Alvise Rigo]

This patch series is based on the previous work [1] and [2] by Rob
Herring and it tries to enhance this work on these points:

- Some of the hardcoded values have been moved to an header file.  This
  header file is also used to share some device structures with the
  mach-virt machine.
- The interrupt-map dt node generation has been revisited; it is now
  done after the generic devices init so that it's possible to attach
  PCI devices by mean of the qdev infrastructure. This allows to have
  several devices in the PCI bus, with the current limitation of one
  interrupt per PCI slot.

Probably the most objectionable part of these patches regards the way
some data and definitions have been shared between the machine and the
device code; a better solution is still under evaluation. Any advice on
this and on the rest of the work is highly appreciated.

This work has been tested attaching several PCI devices to the mach-virt
platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
lsi53c895a and pci-ohci (all attached at the same time).
Even if the original work was not changed in its core functionalities, I
couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
After attaching several qcow2 images, formatting and filling them, I
didn't notice anything wrong. Am I missing something?

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

Alvise Rigo (8):
  mach-virt: move GIC inside mach-virt structure
  mach-virt: improve PCI memory topology definition
  QEMUMachine: finalize_dt function
  generic_pci: create header file
  generic_pci: create own map irq function
  generic_pci: generate dt node after devices init
  generic_pci: realize device with machine data
  generic_pci: add interrupt map structures

 hw/arm/virt.c                     | 110 +++++++++++++++---------
 hw/pci-host/generic-pci.c         | 173 +++++++++++++++++++++++++++++---------
 include/hw/boards.h               |   4 +
 include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
 vl.c                              |   5 ++
 5 files changed, 277 insertions(+), 81 deletions(-)
 create mode 100644 include/hw/pci-host/pci_generic.h

-- 
1.9.1

[Qemu-devel] [RFC PATCH 1/8] mach-virt: move GIC inside mach-virt structure

[Alvise Rigo]

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7122e99..ed9fc7a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -83,6 +83,7 @@ typedef struct VirtBoardInfo {
     void *fdt;
     int fdt_size;
     uint32_t clock_phandle;
+    qemu_irq pic[NUM_IRQS];
 } VirtBoardInfo;
 
 /* Addresses and sizes of our components.
@@ -440,7 +441,6 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void machvirt_init(MachineState *machine)
 {
-    qemu_irq pic[NUM_IRQS];
     MemoryRegion *sysmem = get_system_memory();
     int n;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
@@ -452,6 +452,7 @@ static void machvirt_init(MachineState *machine)
     }
 
     vbi = find_machine_info(cpu_model);
+    qemu_irq *pic = vbi->pic;
 
     if (!vbi) {
         error_report("mach-virt: CPU %s not supported", cpu_model);
-- 
1.9.1

[Qemu-devel] [RFC PATCH 2/8] mach-virt: improve PCI memory topology definition

[Alvise Rigo]

Some of the unnecessary redundancy here will be decreased later in this
series.

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ed9fc7a..c93152f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,7 +42,6 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 
-#define NUM_VIRTIO_TRANSPORTS 32
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 128
@@ -65,8 +64,14 @@ enum {
     VIRT_GIC_DIST,
     VIRT_GIC_CPU,
     VIRT_UART,
+#define VIRT_UART_IRQS 16
     VIRT_MMIO,
+#define NUM_VIRTIO_TRANSPORTS 32
+#define VIRTIO_BASE_IRQ VIRT_UART_IRQS
     VIRT_PCI_CFG,
+    VIRT_PCI_IO,
+    VIRT_PCI_MEM,
+#define PCI_BASE_IRQ (VIRTIO_BASE_IRQ + NUM_VIRTIO_TRANSPORTS - 1)
 };
 
 typedef struct MemMapEntry {
@@ -108,12 +113,15 @@ static const MemMapEntry a15memmap[] = {
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     /* 0x10000000 .. 0x40000000 reserved for PCI */
     [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_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+    [VIRT_MMIO] = VIRTIO_BASE_IRQ, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+    [VIRT_PCI_CFG] = PCI_BASE_IRQ,
 };
 
 static VirtBoardInfo machines[] = {
@@ -351,10 +359,14 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     SysBusDevice *busdev;
     uint32_t gic_phandle;
     char *nodename;
-    hwaddr base = vbi->memmap[VIRT_PCI_CFG].base;
-    hwaddr size = vbi->memmap[VIRT_PCI_CFG].size;
-
-    nodename = g_strdup_printf("/pci@%" PRIx64, base);
+    hwaddr cfg_base = vbi->memmap[VIRT_PCI_CFG].base;
+    hwaddr cfg_size = vbi->memmap[VIRT_PCI_CFG].size;
+    hwaddr io_base = vbi->memmap[VIRT_PCI_IO].base;
+    hwaddr io_size = vbi->memmap[VIRT_PCI_IO].size;
+    hwaddr mem_base = vbi->memmap[VIRT_PCI_MEM].base;
+    hwaddr mem_size = vbi->memmap[VIRT_PCI_MEM].size;
+
+    nodename = g_strdup_printf("/pci@%" PRIx64, cfg_base);
     qemu_fdt_add_subnode(vbi->fdt, nodename);
     qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible",
                             "pci-host-cam-generic");
@@ -363,11 +375,12 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 0x2);
     qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 0x1);
 
-    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, base, 2, size);
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, cfg_base,
+                                                           2, cfg_size);
 
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
-        1, 0x01000000, 2, 0x00000000, 2, 0x11000000, 2, 0x00010000,
-        1, 0x02000000, 2, 0x12000000, 2, 0x12000000, 2, 0x2e000000);
+        1, 0x01000000, 2, 0x00000000, 2, io_base, 2, io_size,
+        1, 0x02000000, 2, 0x12000000, 2, mem_size, 2, mem_size);
 
     gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
@@ -381,9 +394,9 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     dev = qdev_create(NULL, "generic_pci");
     busdev = SYS_BUS_DEVICE(dev);
     qdev_init_nofail(dev);
-    sysbus_mmio_map(busdev, 0, base);       /* PCI config */
-    sysbus_mmio_map(busdev, 1, 0x11000000); /* PCI I/O */
-    sysbus_mmio_map(busdev, 2, 0x12000000); /* PCI memory window */
+    sysbus_mmio_map(busdev, 0, cfg_base); /* PCI config */
+    sysbus_mmio_map(busdev, 1, io_base);  /* PCI I/O */
+    sysbus_mmio_map(busdev, 2, mem_base); /* PCI memory window */
     sysbus_connect_irq(busdev, 0, pic[4]);
     sysbus_connect_irq(busdev, 1, pic[5]);
     sysbus_connect_irq(busdev, 2, pic[6]);
-- 
1.9.1

[Qemu-devel] [RFC PATCH 3/8] QEMUMachine: finalize_dt function

[Alvise Rigo]

Add a new function to be called after that the init of the generic
devices is concluded. This will allow some platforms, like mach-virt, to
conclude its device tree generation disposing of all the information
about attached devices.

Note: This idea accomplishes what is also done by the recent
"[PATCH 0/7] machvirt dynamic sysbus device instantiation", as soon as
those patches will be accepted, this work will be adapted making use of
them.

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 include/hw/boards.h | 4 ++++
 vl.c                | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2d2e2be..40dd003 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -19,12 +19,15 @@ typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
 
 typedef int QEMUMachineGetKvmtypeFunc(const char *arg);
 
+typedef void QEMUMachineFinalizeDtFunc(MachineState *ms);
+
 struct QEMUMachine {
     const char *name;
     const char *alias;
     const char *desc;
     QEMUMachineInitFunc *init;
     QEMUMachineResetFunc *reset;
+    QEMUMachineFinalizeDtFunc *finalize_dt;
     QEMUMachineHotAddCPUFunc *hot_add_cpu;
     QEMUMachineGetKvmtypeFunc *kvm_type;
     BlockInterfaceType block_default_type;
@@ -73,6 +76,7 @@ struct MachineClass {
 
     void (*init)(MachineState *state);
     void (*reset)(void);
+    void (*finalize_dt)(MachineState *state);
     void (*hot_add_cpu)(const int64_t id, Error **errp);
     int (*kvm_type)(const char *arg);
 
diff --git a/vl.c b/vl.c
index ac0e3d7..550a377 100644
--- a/vl.c
+++ b/vl.c
@@ -1596,6 +1596,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
     mc->alias = qm->alias;
     mc->desc = qm->desc;
     mc->init = qm->init;
+    mc->finalize_dt = qm->finalize_dt;
     mc->reset = qm->reset;
     mc->hot_add_cpu = qm->hot_add_cpu;
     mc->kvm_type = qm->kvm_type;
@@ -4456,6 +4457,10 @@ int main(int argc, char **argv, char **envp)
     if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0)
         exit(1);
 
+    if (machine_class->finalize_dt) {
+        machine_class->finalize_dt(current_machine);
+    }
+
     net_check_clients();
 
     ds = init_displaystate();
-- 
1.9.1

[Qemu-devel] [RFC PATCH 4/8] generic_pci: create header file

[Alvise Rigo]

As a matter of fact, the mach-virt platform needs some definitions of
the generic_pci device.

Note: Including the device header file in the mach-virt platform can be
avoided extending properly the idea present in
"[PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices".

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/pci-host/generic-pci.c         | 31 +------------------------------
 include/hw/pci-host/pci_generic.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 30 deletions(-)
 create mode 100644 include/hw/pci-host/pci_generic.h

diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
index 1632e46..8733c67 100644
--- a/hw/pci-host/generic-pci.c
+++ b/hw/pci-host/generic-pci.c
@@ -12,44 +12,15 @@
  */
 
 #include "hw/sysbus.h"
-#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
-#include "hw/pci/pci_host.h"
+#include "hw/pci-host/pci_generic.h"
 #include "exec/address-spaces.h"
 
-typedef struct {
-    PCIHostState parent_obj;
-
-    qemu_irq irq[4];
-    MemoryRegion mem_config;
-    /* Containers representing the PCI address spaces */
-    MemoryRegion pci_io_space;
-    MemoryRegion pci_mem_space;
-    /* Alias regions into PCI address spaces which we expose as sysbus regions.
-     * The offsets into pci_mem_space are controlled by the imap registers.
-     */
-    MemoryRegion pci_io_window;
-    MemoryRegion pci_mem_window;
-    PCIBus pci_bus;
-    PCIDevice pci_dev;
-} PCIVPBState;
-
-
 static const VMStateDescription pci_generic_host_vmstate = {
     .name = "generic-host-pci",
     .version_id = 1,
     .minimum_version_id = 1,
 };
 
-#define TYPE_GENERIC_PCI "generic_pci"
-#define PCI_GEN(obj) \
-    OBJECT_CHECK(PCIVPBState, (obj), TYPE_GENERIC_PCI)
-
-#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
-#define PCI_GEN_HOST(obj) \
-    OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
-
-
 static void pci_cam_config_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned size)
 {
diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
new file mode 100644
index 0000000..46e4cb8
--- /dev/null
+++ b/include/hw/pci-host/pci_generic.h
@@ -0,0 +1,33 @@
+#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"
+
+typedef struct {
+    PCIHostState parent_obj;
+
+    qemu_irq irq[4];
+    MemoryRegion mem_config;
+    /* Containers representing the PCI address spaces */
+    MemoryRegion pci_io_space;
+    MemoryRegion pci_mem_space;
+    /* Alias regions into PCI address spaces which we expose as sysbus regions.
+     * The offsets into pci_mem_space are controlled by the imap registers.
+     */
+    MemoryRegion pci_io_window;
+    MemoryRegion pci_mem_window;
+    PCIBus pci_bus;
+    PCIDevice pci_dev;
+} PCIVPBState;
+
+#define TYPE_GENERIC_PCI "generic_pci"
+#define PCI_GEN(obj) \
+    OBJECT_CHECK(PCIVPBState, (obj), TYPE_GENERIC_PCI)
+
+#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
+#define PCI_GEN_HOST(obj) \
+    OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
+
+#endif
-- 
1.9.1

[Qemu-devel] [RFC PATCH 5/8] generic_pci: create own map irq function

[Alvise Rigo]

Allocate MAX_PCI_DEVICES IRQs for the pci_generic device.  For now we
assume that at every PCI slot corresponds an interrupt line: this also
seems what the kernel code does when parsing the interrupt mapping from
the device tree.  At the moment there isn't a structure that keeps track
of the IRQ mapping; it will be added later by a patch of this series.

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/arm/virt.c                     | 10 ++++++----
 hw/pci-host/generic-pci.c         | 15 ++++++++++++---
 include/hw/pci-host/pci_generic.h |  4 +++-
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c93152f..a433902 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -41,6 +41,7 @@
 #include "exec/address-spaces.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
+#include "hw/pci-host/pci_generic.h"
 
 
 /* Number of external interrupt lines to configure the GIC with */
@@ -359,6 +360,7 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     SysBusDevice *busdev;
     uint32_t gic_phandle;
     char *nodename;
+    int i;
     hwaddr cfg_base = vbi->memmap[VIRT_PCI_CFG].base;
     hwaddr cfg_size = vbi->memmap[VIRT_PCI_CFG].size;
     hwaddr io_base = vbi->memmap[VIRT_PCI_IO].base;
@@ -397,10 +399,10 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     sysbus_mmio_map(busdev, 0, cfg_base); /* PCI config */
     sysbus_mmio_map(busdev, 1, io_base);  /* PCI I/O */
     sysbus_mmio_map(busdev, 2, mem_base); /* PCI memory window */
-    sysbus_connect_irq(busdev, 0, pic[4]);
-    sysbus_connect_irq(busdev, 1, pic[5]);
-    sysbus_connect_irq(busdev, 2, pic[6]);
-    sysbus_connect_irq(busdev, 3, pic[7]);
+
+    for ( i = 0; i < MAX_PCI_DEVICES; i++) {
+        sysbus_connect_irq(busdev, i, pic[vbi->irqmap[VIRT_PCI_CFG] + i]);
+    }
 
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
     pci_create_simple(pci_bus, -1, "pci-ohci");
diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
index 8733c67..75aae45 100644
--- a/hw/pci-host/generic-pci.c
+++ b/hw/pci-host/generic-pci.c
@@ -65,18 +65,27 @@ static void pci_generic_host_init(Object *obj)
     qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
 }
 
+static int generic_pci_map_irq_fn(PCIDevice *pci_dev, int pin)
+{
+    if (!pin) {
+        return PCI_SLOT(pci_dev->devfn);
+    }
+
+    hw_error("generic_pci: only one pin per device supported.");
+}
+
 static void pci_generic_host_realize(DeviceState *dev, Error **errp)
 {
     PCIVPBState *s = PCI_GEN(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     int i;
 
-    for (i = 0; i < 4; i++) {
+    for (i = 0; i < MAX_PCI_DEVICES; 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, 4);
+    pci_bus_irqs(&s->pci_bus, pci_generic_set_irq, generic_pci_map_irq_fn,
+                 s->irq, MAX_PCI_DEVICES);
 
     /* Our memory regions are:
      * 0 : PCI config window
diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
index 46e4cb8..2367d80 100644
--- a/include/hw/pci-host/pci_generic.h
+++ b/include/hw/pci-host/pci_generic.h
@@ -5,10 +5,12 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_host.h"
 
+#define MAX_PCI_DEVICES 10
+
 typedef struct {
     PCIHostState parent_obj;
 
-    qemu_irq irq[4];
+    qemu_irq irq[MAX_PCI_DEVICES];
     MemoryRegion mem_config;
     /* Containers representing the PCI address spaces */
     MemoryRegion pci_io_space;
-- 
1.9.1

[Qemu-devel] [RFC PATCH 6/8] generic_pci: generate dt node after devices init

[Alvise Rigo]

Keeping advantage of the finalize_dt QEMUMachine function, the mach-virt
machine now completes the device tree creation after that all the
generic devices have been instantiated. This allows to generate the
interrupt-map node according to the devices attached to the PCI bus.
These devices can be specified either as command line argument (like
-device lsi53c895a addr=0x5 ...) or explicitly inside the machine
definition with the pci_create_simple method.

Fill also the generic_pci state with the offsets and sizes of the memory
regions needed by the PCI system. The offset in the machine address
space of these regions (config, IO and memory) is specified by the
mach-virt platform.

TODO:
- Part of the ranges device node is still hardcoded.

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/arm/virt.c                     | 80 +++++++++++++++++++--------------
 hw/pci-host/generic-pci.c         | 94 +++++++++++++++++++++++++++++++++++++++
 include/hw/pci-host/pci_generic.h | 20 +++++++++
 3 files changed, 160 insertions(+), 34 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a433902..e182282 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -358,44 +358,35 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     PCIBus *pci_bus;
     DeviceState *dev;
     SysBusDevice *busdev;
-    uint32_t gic_phandle;
-    char *nodename;
-    int i;
+    PCIVPBState *ps;
+    int i, count = 0;
     hwaddr cfg_base = vbi->memmap[VIRT_PCI_CFG].base;
-    hwaddr cfg_size = vbi->memmap[VIRT_PCI_CFG].size;
     hwaddr io_base = vbi->memmap[VIRT_PCI_IO].base;
-    hwaddr io_size = vbi->memmap[VIRT_PCI_IO].size;
     hwaddr mem_base = vbi->memmap[VIRT_PCI_MEM].base;
-    hwaddr mem_size = vbi->memmap[VIRT_PCI_MEM].size;
-
-    nodename = g_strdup_printf("/pci@%" PRIx64, 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);
-
-    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, cfg_base,
-                                                           2, cfg_size);
-
-    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
-        1, 0x01000000, 2, 0x00000000, 2, io_base, 2, io_size,
-        1, 0x02000000, 2, 0x12000000, 2, mem_size, 2, mem_size);
-
-    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
-    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
-        1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);
-    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map",
-        1, 0x0000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x4, 1, 0x1,
-        1, 0x0800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x5, 1, 0x1,
-        1, 0x1000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x6, 1, 0x1,
-        1, 0x1800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x7, 1, 0x1);
 
     dev = qdev_create(NULL, "generic_pci");
     busdev = SYS_BUS_DEVICE(dev);
+    ps = PCI_GEN(dev);
+
+    /* Set the mapping data in the device structure where:
+     * ptr[i] = base address
+     * ptr[i+1] = size
+     * i = 0 => config
+     * i = 2 => I/O
+     * i = 4 => memory
+     * These values are needed by the specific device code.
+     * */
+    hwaddr *ptr = g_malloc(6 * sizeof(hwaddr));
+    for (i = VIRT_PCI_CFG; i <= VIRT_PCI_MEM; i++) {
+        ptr[count++] = vbi->memmap[i].base;
+        ptr[count++] = vbi->memmap[i].size;
+    }
+    ps->dt_data.fdt = vbi->fdt;
+    ps->dt_data.irq_base = vbi->irqmap[VIRT_PCI_CFG];
+    ps->dt_data.addr_mapping = ptr;
+
     qdev_init_nofail(dev);
+
     sysbus_mmio_map(busdev, 0, cfg_base); /* PCI config */
     sysbus_mmio_map(busdev, 1, io_base);  /* PCI I/O */
     sysbus_mmio_map(busdev, 2, mem_base); /* PCI memory window */
@@ -407,8 +398,6 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
     pci_create_simple(pci_bus, -1, "pci-ohci");
     pci_create_simple(pci_bus, -1, "lsi53c895a");
-
-    g_free(nodename);
 }
 
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -454,6 +443,29 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
     return board->fdt;
 }
 
+static void machvirt_finalize_dt(MachineState *machine)
+{
+    BusState *bus;
+    BusChild *child;
+    VirtBoardInfo *vbi;
+    /* TODO store somewhere the CPU model used */
+    vbi = find_machine_info("cortex-a15");
+
+    /* If the device has been successfully created, create also its
+     * device node in the dt. */
+    bus = sysbus_get_default();
+    QTAILQ_FOREACH(child, &bus->children, sibling) {
+        DeviceState *dev = child->child;
+        if (!strcmp(object_get_typename(OBJECT(dev)), TYPE_GENERIC_PCI)) {
+            GenericPCIClass *gpci = GENERIC_PCI_GET_CLASS(OBJECT(dev));
+            /* create device tree node */
+            gpci->gen_dt_node(dev);
+        }
+    }
+
+    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
+}
+
 static void machvirt_init(MachineState *machine)
 {
     MemoryRegion *sysmem = get_system_memory();
@@ -542,13 +554,13 @@ static void machvirt_init(MachineState *machine)
     vbi->bootinfo.board_id = -1;
     vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
     vbi->bootinfo.get_dtb = machvirt_dtb;
-    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
 }
 
 static QEMUMachine machvirt_a15_machine = {
     .name = "virt",
     .desc = "ARM Virtual Machine",
     .init = machvirt_init,
+    .finalize_dt = machvirt_finalize_dt,
     .max_cpus = 4,
 };
 
diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
index 75aae45..2068079 100644
--- a/hw/pci-host/generic-pci.c
+++ b/hw/pci-host/generic-pci.c
@@ -14,6 +14,7 @@
 #include "hw/sysbus.h"
 #include "hw/pci-host/pci_generic.h"
 #include "exec/address-spaces.h"
+#include "sysemu/device_tree.h"
 
 static const VMStateDescription pci_generic_host_vmstate = {
     .name = "generic-host-pci",
@@ -130,6 +131,96 @@ static void pci_generic_host_class_init(ObjectClass *klass, void *data)
     dc->cannot_instantiate_with_device_add_yet = true;
 }
 
+struct dt_irq_mapping {
+        DeviceState *dev;
+        uint32_t gic_phandle;
+        int base_irq_num;
+        uint64_t *data;
+};
+
+#define IRQ_MAPPING_CELLS 14
+/* Generate the irq_mapping data and return the number of the device attached
+ * to the device bus.
+ * */
+static int generate_int_mapping(struct dt_irq_mapping *irq_map)
+{
+    BusState *inner_bus;
+    BusChild *inner;
+    int num_slots = 0;
+    uint64_t *data_ptr = irq_map->data;
+
+    QLIST_FOREACH(inner_bus, &irq_map->dev->child_bus, sibling) {
+        QTAILQ_FOREACH(inner, &inner_bus->children, sibling) {
+            DeviceState *dev = inner->child;
+            PCIDevice *pdev = PCI_DEVICE(dev);
+            int pci_slot = PCI_SLOT(pdev->devfn);
+
+            uint64_t buffer[IRQ_MAPPING_CELLS] =
+            {1, pci_slot << 11, 2, 0x00000000, 1, 0x1,
+             1, irq_map->gic_phandle, 1, 0, 1, irq_map->base_irq_num + pci_slot,
+             1, 0x1};
+
+            memcpy(data_ptr, buffer, IRQ_MAPPING_CELLS * sizeof(*buffer));
+            num_slots++;
+            data_ptr += IRQ_MAPPING_CELLS;
+        }
+    }
+
+    return num_slots;
+}
+
+static void generate_dt_node(DeviceState *dev)
+{
+    PCIVPBState *s = PCI_GEN(dev);
+    char *nodename;
+    uint32_t gic_phandle;
+    int num_dev;
+    hwaddr *map = s->dt_data.addr_mapping;
+    void *fdt = s->dt_data.fdt;
+
+    nodename = g_strdup_printf("/pci@%" PRIx64, map[0]);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                            "pci-host-cam-generic");
+    qemu_fdt_setprop_string(fdt, nodename, "device_type", "pci");
+    qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 0x3);
+    qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0x1);
+
+    /* config space */
+    qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", 2, map[0],
+                                                            2, map[1]);
+
+    qemu_fdt_setprop_sized_cells(fdt, nodename, "ranges",
+         1, 0x01000000, 2, 0x00000000, 2, map[2], 2, map[3],
+         1, 0x02000000, 2, 0x12000000, 2, map[4], 2, map[5]);
+
+    gic_phandle = qemu_fdt_get_phandle(fdt, "/intc");
+    qemu_fdt_setprop_sized_cells(fdt, nodename, "interrupt-map-mask",
+                                  1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);
+
+    /* Generate the interrupt mapping according to the devices attached
+     * to the PCI bus of the device. */
+    uint64_t *int_mapping_data = g_malloc0(IRQ_MAPPING_CELLS * sizeof(uint64_t)
+                                                            * MAX_PCI_DEVICES);
+
+    struct dt_irq_mapping dt_map = {
+        .dev = dev,
+        .gic_phandle = gic_phandle,
+        .base_irq_num = s->dt_data.irq_base,
+        .data = int_mapping_data
+    };
+
+    num_dev = generate_int_mapping(&dt_map);
+    qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "interrupt-map",
+                        (num_dev * IRQ_MAPPING_CELLS)/2, int_mapping_data);
+
+    g_free(int_mapping_data);
+    g_free(nodename);
+    /* Once the dt node is created, this data is no longer necessary */
+    g_free(s->dt_data.addr_mapping);
+}
+
 static const TypeInfo pci_generic_host_info = {
     .name          = TYPE_GENERIC_PCI_HOST,
     .parent        = TYPE_PCI_DEVICE,
@@ -140,9 +231,11 @@ static const TypeInfo pci_generic_host_info = {
 static void pci_generic_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    GenericPCIClass *pc = GENERIC_PCI_CLASS(klass);
 
     dc->realize = pci_generic_host_realize;
     dc->vmsd = &pci_generic_host_vmstate;
+    pc->gen_dt_node = generate_dt_node;
 }
 
 static const TypeInfo pci_generic_info = {
@@ -151,6 +244,7 @@ static const TypeInfo pci_generic_info = {
     .instance_size = sizeof(PCIVPBState),
     .instance_init = pci_generic_host_init,
     .class_init    = pci_generic_class_init,
+    .class_size    = sizeof(GenericPCIClass),
 };
 
 static void generic_pci_host_register_types(void)
diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
index 2367d80..039549f 100644
--- a/include/hw/pci-host/pci_generic.h
+++ b/include/hw/pci-host/pci_generic.h
@@ -7,6 +7,12 @@
 
 #define MAX_PCI_DEVICES 10
 
+struct dt_data {
+    void *fdt;
+    int irq_base;
+    hwaddr *addr_mapping;
+};
+
 typedef struct {
     PCIHostState parent_obj;
 
@@ -22,8 +28,17 @@ typedef struct {
     MemoryRegion pci_mem_window;
     PCIBus pci_bus;
     PCIDevice pci_dev;
+    /* Device tree data set by the machine
+     */
+    struct dt_data dt_data;
 } PCIVPBState;
 
+typedef struct GenericPCIClass {
+    PCIDeviceClass parent_class;
+
+    void (*gen_dt_node)(DeviceState *dev);
+} GenericPCIClass;
+
 #define TYPE_GENERIC_PCI "generic_pci"
 #define PCI_GEN(obj) \
     OBJECT_CHECK(PCIVPBState, (obj), TYPE_GENERIC_PCI)
@@ -32,4 +47,9 @@ typedef struct {
 #define PCI_GEN_HOST(obj) \
     OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
 
+#define GENERIC_PCI_CLASS(klass) \
+     OBJECT_CLASS_CHECK(GenericPCIClass, (klass), TYPE_GENERIC_PCI)
+#define GENERIC_PCI_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(GenericPCIClass, (obj), TYPE_GENERIC_PCI)
+
 #endif
-- 
1.9.1

[Qemu-devel] [RFC PATCH 7/8] generic_pci: realize device with machine data

[Alvise Rigo]

Realize the device according to the offsets specified by the machine
memory map.

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/pci-host/generic-pci.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
index 2068079..1bde69c 100644
--- a/hw/pci-host/generic-pci.c
+++ b/hw/pci-host/generic-pci.c
@@ -79,6 +79,7 @@ static void pci_generic_host_realize(DeviceState *dev, Error **errp)
 {
     PCIVPBState *s = PCI_GEN(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    hwaddr *mapping_data;
     int i;
 
     for (i = 0; i < MAX_PCI_DEVICES; i++) {
@@ -92,16 +93,19 @@ static void pci_generic_host_realize(DeviceState *dev, Error **errp)
      * 0 : PCI config window
      * 1 : PCI IO window
      * 2 : PCI memory windows
+     * The CPU addresses and size of each memory region have been set by the
+     * machine code inside s->dt_data.
      */
+    mapping_data = s->dt_data.addr_mapping;
     memory_region_init_io(&s->mem_config, OBJECT(s), &pci_vpb_config_ops, s,
-                          "pci-config", 0x1000000);
+                          "pci-config", mapping_data[1]);
     sysbus_init_mmio(sbd, &s->mem_config);
 
     /* The window into I/O space is always into a fixed base address;
      * its size is the same for both realview and versatile.
      */
     memory_region_init_alias(&s->pci_io_window, OBJECT(s), "pci-io-win",
-                             &s->pci_io_space, 0, 0x10000);
+                    &s->pci_io_space, mapping_data[2], mapping_data[3]);
     sysbus_init_mmio(sbd, &s->pci_io_space);
 
     /* Create the alias regions corresponding to our three windows onto
@@ -109,7 +113,7 @@ static void pci_generic_host_realize(DeviceState *dev, Error **errp)
      * offsets are guest controllable via the IMAP registers.
      */
     memory_region_init_alias(&s->pci_mem_window, OBJECT(s), "pci-mem-win",
-                             &s->pci_mem_space, 0x12000000, 0x2e000000);
+                     &s->pci_mem_space, mapping_data[4], mapping_data[5]);
     sysbus_init_mmio(sbd, &s->pci_mem_window);
 
     /* TODO Remove once realize propagates to child devices. */
-- 
1.9.1

[Qemu-devel] [RFC PATCH 8/8] generic_pci: add interrupt map structures

[Alvise Rigo]

Create a generic_pci_host state to include the IRQ map to be used when
resolving the PCI interrupts. These structures can be useful to support
more complicated scenarios, like with multi functions PCI devices.

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/pci-host/generic-pci.c         | 37 ++++++++++++++++++++++++++-----------
 include/hw/pci-host/pci_generic.h | 15 +++++++++++++--
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
index 1bde69c..24c0ce8 100644
--- a/hw/pci-host/generic-pci.c
+++ b/hw/pci-host/generic-pci.c
@@ -62,14 +62,19 @@ static void pci_generic_host_init(Object *obj)
                         PCI_DEVFN(0, 0), TYPE_PCIE_BUS);
     h->bus = &s->pci_bus;
 
-    object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_GENERIC_PCI_HOST);
-    qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
+    object_initialize(&s->pci_gen, sizeof(s->pci_gen), TYPE_GENERIC_PCI_HOST);
+    qdev_set_parent_bus(DEVICE(&s->pci_gen), BUS(&s->pci_bus));
 }
 
 static int generic_pci_map_irq_fn(PCIDevice *pci_dev, int pin)
 {
+    BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
+    PCIBus *pci_bus = PCI_BUS(bus);
+    PCIDevice *pdev = pci_bus->devices[PCI_DEVFN(0, 0)];
+    GenericPCIHostState *gps = PCI_GEN_HOST(pdev);
+
     if (!pin) {
-        return PCI_SLOT(pci_dev->devfn);
+        return gps->irqmap.slot_idx_map[PCI_SLOT(pci_dev->devfn)];
     }
 
     hw_error("generic_pci: only one pin per device supported.");
@@ -117,7 +122,7 @@ static void pci_generic_host_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &s->pci_mem_window);
 
     /* TODO Remove once realize propagates to child devices. */
-    object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp);
+    object_property_set_bool(OBJECT(&s->pci_gen), true, "realized", errp);
 }
 
 static void pci_generic_host_class_init(ObjectClass *klass, void *data)
@@ -146,11 +151,11 @@ struct dt_irq_mapping {
 /* Generate the irq_mapping data and return the number of the device attached
  * to the device bus.
  * */
-static int generate_int_mapping(struct dt_irq_mapping *irq_map)
+static int generate_int_mapping(struct dt_irq_mapping *irq_map, PCIVPBState *s)
 {
     BusState *inner_bus;
     BusChild *inner;
-    int num_slots = 0;
+    int slot_count = 0;
     uint64_t *data_ptr = irq_map->data;
 
     QLIST_FOREACH(inner_bus, &irq_map->dev->child_bus, sibling) {
@@ -158,19 +163,29 @@ static int generate_int_mapping(struct dt_irq_mapping *irq_map)
             DeviceState *dev = inner->child;
             PCIDevice *pdev = PCI_DEVICE(dev);
             int pci_slot = PCI_SLOT(pdev->devfn);
+            uint8_t *slot_idx = s->pci_gen.irqmap.slot_idx_map;
+            uint8_t *slot_irq = s->pci_gen.irqmap.slot_irq_map;
+
+            if (slot_count > MAX_PCI_DEVICES) {
+                hw_error("generic_pci: too many PCI devices.");
+            }
+
+            /* Every PCI slot has one interrupt mapped. */
+            slot_idx[pci_slot] = slot_count;
+            slot_irq[slot_count] = irq_map->base_irq_num + slot_count;
 
             uint64_t buffer[IRQ_MAPPING_CELLS] =
             {1, pci_slot << 11, 2, 0x00000000, 1, 0x1,
-             1, irq_map->gic_phandle, 1, 0, 1, irq_map->base_irq_num + pci_slot,
+             1, irq_map->gic_phandle, 1, 0, 1, slot_irq[slot_count],
              1, 0x1};
 
             memcpy(data_ptr, buffer, IRQ_MAPPING_CELLS * sizeof(*buffer));
-            num_slots++;
+            slot_count++;
             data_ptr += IRQ_MAPPING_CELLS;
         }
     }
 
-    return num_slots;
+    return slot_count;
 }
 
 static void generate_dt_node(DeviceState *dev)
@@ -215,7 +230,7 @@ static void generate_dt_node(DeviceState *dev)
         .data = int_mapping_data
     };
 
-    num_dev = generate_int_mapping(&dt_map);
+    num_dev = generate_int_mapping(&dt_map, s);
     qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "interrupt-map",
                         (num_dev * IRQ_MAPPING_CELLS)/2, int_mapping_data);
 
@@ -228,7 +243,7 @@ static void generate_dt_node(DeviceState *dev)
 static const TypeInfo pci_generic_host_info = {
     .name          = TYPE_GENERIC_PCI_HOST,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIDevice),
+    .instance_size = sizeof(GenericPCIHostState),
     .class_init    = pci_generic_host_class_init,
 };
 
diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
index 039549f..c65bc6c 100644
--- a/include/hw/pci-host/pci_generic.h
+++ b/include/hw/pci-host/pci_generic.h
@@ -14,6 +14,17 @@ struct dt_data {
 };
 
 typedef struct {
+    PCIDevice parent_obj;
+
+    struct irqmap {
+        /* slot_idx_map[i] = index inside slot_irq_map for device at slot i */
+        uint8_t slot_idx_map[PCI_SLOT_MAX];
+        /* slot_irq_map[i] = irq num. of the i-th device attached to the bus */
+        uint8_t slot_irq_map[MAX_PCI_DEVICES];
+    } irqmap;
+} GenericPCIHostState;
+
+typedef struct {
     PCIHostState parent_obj;
 
     qemu_irq irq[MAX_PCI_DEVICES];
@@ -27,7 +38,7 @@ typedef struct {
     MemoryRegion pci_io_window;
     MemoryRegion pci_mem_window;
     PCIBus pci_bus;
-    PCIDevice pci_dev;
+    GenericPCIHostState pci_gen;
     /* Device tree data set by the machine
      */
     struct dt_data dt_data;
@@ -45,7 +56,7 @@ typedef struct GenericPCIClass {
 
 #define TYPE_GENERIC_PCI_HOST "generic_pci_host"
 #define PCI_GEN_HOST(obj) \
-    OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
+    OBJECT_CHECK(GenericPCIHostState, (obj), TYPE_GENERIC_PCI_HOST)
 
 #define GENERIC_PCI_CLASS(klass) \
      OBJECT_CLASS_CHECK(GenericPCIClass, (klass), TYPE_GENERIC_PCI)
-- 
1.9.1

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

[Peter Maydell]

On 11 July 2014 08:21, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
> This work has been tested attaching several PCI devices to the mach-virt
> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
> lsi53c895a and pci-ohci (all attached at the same time).
> Even if the original work was not changed in its core functionalities, I
> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
> After attaching several qcow2 images, formatting and filling them, I
> didn't notice anything wrong. Am I missing something?

Interesting. Perhaps the bug was on the kernel side; which
guest kernel version are you using to test with?

thanks
-- PMM

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

[Alvise Rigo]

The kernel version is a very recent one: v3.16.0-rc1.
Maybe you are right. I will test some older version to see if I'm able
to reproduce the issue.

Thank you,
alvise


Il 11/07/2014 11:09, Peter Maydell ha scritto:
> On 11 July 2014 08:21, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
>> This work has been tested attaching several PCI devices to the mach-virt
>> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
>> lsi53c895a and pci-ohci (all attached at the same time).
>> Even if the original work was not changed in its core functionalities, I
>> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
>> After attaching several qcow2 images, formatting and filling them, I
>> didn't notice anything wrong. Am I missing something?
> 
> Interesting. Perhaps the bug was on the kernel side; which
> guest kernel version are you using to test with?
> 
> thanks
> -- PMM
> 

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

[Rob Herring]

On Fri, Jul 11, 2014 at 4:28 AM, Alvise Rigo
<a.rigo@virtualopensystems.com> wrote:
> The kernel version is a very recent one: v3.16.0-rc1.
> Maybe you are right. I will test some older version to see if I'm able
> to reproduce the issue.

BTW, the lsi driver has compile time option to use i/o or memory
accesses. I usually test both modes when doing PCI related changes.
Unfortunately, I don't recall whether the problem I saw was only for
one or both.

Rob

>
> Thank you,
> alvise
>
>
> Il 11/07/2014 11:09, Peter Maydell ha scritto:
>> On 11 July 2014 08:21, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
>>> This work has been tested attaching several PCI devices to the mach-virt
>>> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
>>> lsi53c895a and pci-ohci (all attached at the same time).
>>> Even if the original work was not changed in its core functionalities, I
>>> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
>>> After attaching several qcow2 images, formatting and filling them, I
>>> didn't notice anything wrong. Am I missing something?
>>
>> Interesting. Perhaps the bug was on the kernel side; which
>> guest kernel version are you using to test with?
>>
>> thanks
>> -- PMM
>>

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

[Claudio Fontana]

On 11.07.2014 11:28, Alvise Rigo wrote:
> The kernel version is a very recent one: v3.16.0-rc1.
> Maybe you are right. I will test some older version to see if I'm able
> to reproduce the issue.
> 
> Thank you,
> alvise

Any news on this?

I will be soon in the situation when I can start testing these as the way to get PCI working in OSv for AArch64,
but will require a bit more time, since there is some more mechanical work involved.

Ciao,

Claudio

> Il 11/07/2014 11:09, Peter Maydell ha scritto:
>> On 11 July 2014 08:21, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
>>> This work has been tested attaching several PCI devices to the mach-virt
>>> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
>>> lsi53c895a and pci-ohci (all attached at the same time).
>>> Even if the original work was not changed in its core functionalities, I
>>> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
>>> After attaching several qcow2 images, formatting and filling them, I
>>> didn't notice anything wrong. Am I missing something?
>>
>> Interesting. Perhaps the bug was on the kernel side; which
>> guest kernel version are you using to test with?
>>
>> thanks
>> -- PMM
>>
> 

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

[alvise rigo]

Hello Claudio,

Unfortunately I'm still not able to reproduce the problem.

I suspect though that the issue concerns the way the tests were conducted, so
I take this opportunity to ask Rob how many disks you used to test the initial
patch series.
I used something like:

-device lsi53c895a -drive if=scsi,index=1,file=scsi.img \
        -drive if=scsi,index=2,file=scsi_2.img ...

to attach the SCSI disks to the guest.

Thank you,
alvise

On Tue, Sep 9, 2014 at 6:35 PM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> On 11.07.2014 11:28, Alvise Rigo wrote:
>> The kernel version is a very recent one: v3.16.0-rc1.
>> Maybe you are right. I will test some older version to see if I'm able
>> to reproduce the issue.
>>
>> Thank you,
>> alvise
>
> Any news on this?
>
> I will be soon in the situation when I can start testing these as the way to get PCI working in OSv for AArch64,
> but will require a bit more time, since there is some more mechanical work involved.
>
> Ciao,
>
> Claudio
>
>> Il 11/07/2014 11:09, Peter Maydell ha scritto:
>>> On 11 July 2014 08:21, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
>>>> This work has been tested attaching several PCI devices to the mach-virt
>>>> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
>>>> lsi53c895a and pci-ohci (all attached at the same time).
>>>> Even if the original work was not changed in its core functionalities, I
>>>> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
>>>> After attaching several qcow2 images, formatting and filling them, I
>>>> didn't notice anything wrong. Am I missing something?
>>>
>>> Interesting. Perhaps the bug was on the kernel side; which
>>> guest kernel version are you using to test with?
>>>
>>> thanks
>>> -- PMM
>>>
>>
>
>
>

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

[Claudio Fontana]

Hi Alvise,

On 11.07.2014 09:21, Alvise Rigo wrote:
> This patch series is based on the previous work [1] and [2] by Rob
> Herring and it tries to enhance this work on these points:

do your patches need to be applied on top of Rob's?

I ask because I cannot see the patch "hw/arm/virt: Add generic PCI host device" and among these,
as wll as the "hw/pci-host: add a generic PCI host".

I think those two need modifications as well, as they hardcode addresses, and also try to register
the PCI bus as a PCIE bus: does it really provide a PCI-Express? (probably harmless but still would benefit from review).

> 
> - Some of the hardcoded values have been moved to an header file.  This
>   header file is also used to share some device structures with the
>   mach-virt machine.

Some additional hardcoded addresses have been also introduced with your changes though.
(I'll post comments to the the patches in the series momentarily).

> - The interrupt-map dt node generation has been revisited; it is now
>   done after the generic devices init so that it's possible to attach
>   PCI devices by mean of the qdev infrastructure. This allows to have
>   several devices in the PCI bus, with the current limitation of one
>   interrupt per PCI slot.
> 
> Probably the most objectionable part of these patches regards the way
> some data and definitions have been shared between the machine and the
> device code; a better solution is still under evaluation. Any advice on
> this and on the rest of the work is highly appreciated.
> 
> This work has been tested attaching several PCI devices to the mach-virt
> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
> lsi53c895a and pci-ohci (all attached at the same time).
> Even if the original work was not changed in its core functionalities, I
> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
> After attaching several qcow2 images, formatting and filling them, I
> didn't notice anything wrong. Am I missing something?
> 
> 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
> 
> Alvise Rigo (8):
>   mach-virt: move GIC inside mach-virt structure
>   mach-virt: improve PCI memory topology definition
>   QEMUMachine: finalize_dt function
>   generic_pci: create header file
>   generic_pci: create own map irq function
>   generic_pci: generate dt node after devices init
>   generic_pci: realize device with machine data
>   generic_pci: add interrupt map structures
> 
>  hw/arm/virt.c                     | 110 +++++++++++++++---------
>  hw/pci-host/generic-pci.c         | 173 +++++++++++++++++++++++++++++---------
>  include/hw/boards.h               |   4 +
>  include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
>  vl.c                              |   5 ++
>  5 files changed, 277 insertions(+), 81 deletions(-)
>  create mode 100644 include/hw/pci-host/pci_generic.h
> 

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

[alvise rigo]

On Wed, Nov 5, 2014 at 11:23 AM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> Hi Alvise,

Hi Claudio,

>
> On 11.07.2014 09:21, Alvise Rigo wrote:
>> This patch series is based on the previous work [1] and [2] by Rob
>> Herring and it tries to enhance this work on these points:
>
> do your patches need to be applied on top of Rob's?

Yes, definitively.

>
> I ask because I cannot see the patch "hw/arm/virt: Add generic PCI host device" and among these,
> as wll as the "hw/pci-host: add a generic PCI host".

It seems to me that it's common practice in this mailing list to not
include the patches upon which the work is based.
What I could do for the next version is to set up a repository with
all the dependences included.

>
> I think those two need modifications as well, as they hardcode addresses, and also try to register
> the PCI bus as a PCIE bus: does it really provide a PCI-Express? (probably harmless but still would benefit from review).

This patches should not be compatible with PCI-Express devices, since
for them the ECAM access mechanism is required.
The initial idea was to add a second bus for PCI-Express cards,
however there wouldn't have been devices to use/test it, so I put this
on hold.
In any case, I will consider it for the next iteration.

Regards,
alvise

>
>>
>> - Some of the hardcoded values have been moved to an header file.  This
>>   header file is also used to share some device structures with the
>>   mach-virt machine.
>
> Some additional hardcoded addresses have been also introduced with your changes though.
> (I'll post comments to the the patches in the series momentarily).
>
>> - The interrupt-map dt node generation has been revisited; it is now
>>   done after the generic devices init so that it's possible to attach
>>   PCI devices by mean of the qdev infrastructure. This allows to have
>>   several devices in the PCI bus, with the current limitation of one
>>   interrupt per PCI slot.
>>
>> Probably the most objectionable part of these patches regards the way
>> some data and definitions have been shared between the machine and the
>> device code; a better solution is still under evaluation. Any advice on
>> this and on the rest of the work is highly appreciated.
>>
>> This work has been tested attaching several PCI devices to the mach-virt
>> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
>> lsi53c895a and pci-ohci (all attached at the same time).
>> Even if the original work was not changed in its core functionalities, I
>> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
>> After attaching several qcow2 images, formatting and filling them, I
>> didn't notice anything wrong. Am I missing something?
>>
>> 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
>>
>> Alvise Rigo (8):
>>   mach-virt: move GIC inside mach-virt structure
>>   mach-virt: improve PCI memory topology definition
>>   QEMUMachine: finalize_dt function
>>   generic_pci: create header file
>>   generic_pci: create own map irq function
>>   generic_pci: generate dt node after devices init
>>   generic_pci: realize device with machine data
>>   generic_pci: add interrupt map structures
>>
>>  hw/arm/virt.c                     | 110 +++++++++++++++---------
>>  hw/pci-host/generic-pci.c         | 173 +++++++++++++++++++++++++++++---------
>>  include/hw/boards.h               |   4 +
>>  include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
>>  vl.c                              |   5 ++
>>  5 files changed, 277 insertions(+), 81 deletions(-)
>>  create mode 100644 include/hw/pci-host/pci_generic.h
>>
>
>

Re: [Qemu-devel] [RFC PATCH 6/8] generic_pci: generate dt node after devices init

[Claudio Fontana]

On 11.07.2014 09:21, Alvise Rigo wrote:
> Keeping advantage of the finalize_dt QEMUMachine function, the mach-virt
> machine now completes the device tree creation after that all the
> generic devices have been instantiated. This allows to generate the
> interrupt-map node according to the devices attached to the PCI bus.
> These devices can be specified either as command line argument (like
> -device lsi53c895a addr=0x5 ...) or explicitly inside the machine
> definition with the pci_create_simple method.
> 
> Fill also the generic_pci state with the offsets and sizes of the memory
> regions needed by the PCI system. The offset in the machine address
> space of these regions (config, IO and memory) is specified by the
> mach-virt platform.

If this is mach-virt specific, why is this called generic_pci..?
Is it generic ARM pci?

> 
> TODO:
> - Part of the ranges device node is still hardcoded.

Also this would benefit from extensive commenting.
Especially the exact meaning of the interrupt mapping, and if it is ARM specific,
how the interrupts end up being mapped to gic irqs.
[see below]

> 
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>  hw/arm/virt.c                     | 80 +++++++++++++++++++--------------
>  hw/pci-host/generic-pci.c         | 94 +++++++++++++++++++++++++++++++++++++++
>  include/hw/pci-host/pci_generic.h | 20 +++++++++
>  3 files changed, 160 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a433902..e182282 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -358,44 +358,35 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
>      PCIBus *pci_bus;
>      DeviceState *dev;
>      SysBusDevice *busdev;
> -    uint32_t gic_phandle;
> -    char *nodename;
> -    int i;
> +    PCIVPBState *ps;
> +    int i, count = 0;
>      hwaddr cfg_base = vbi->memmap[VIRT_PCI_CFG].base;
> -    hwaddr cfg_size = vbi->memmap[VIRT_PCI_CFG].size;
>      hwaddr io_base = vbi->memmap[VIRT_PCI_IO].base;
> -    hwaddr io_size = vbi->memmap[VIRT_PCI_IO].size;
>      hwaddr mem_base = vbi->memmap[VIRT_PCI_MEM].base;
> -    hwaddr mem_size = vbi->memmap[VIRT_PCI_MEM].size;
> -
> -    nodename = g_strdup_printf("/pci@%" PRIx64, 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);
> -
> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, cfg_base,
> -                                                           2, cfg_size);
> -
> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
> -        1, 0x01000000, 2, 0x00000000, 2, io_base, 2, io_size,
> -        1, 0x02000000, 2, 0x12000000, 2, mem_size, 2, mem_size);
> -
> -    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
> -        1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);
> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map",
> -        1, 0x0000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x4, 1, 0x1,
> -        1, 0x0800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x5, 1, 0x1,
> -        1, 0x1000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x6, 1, 0x1,
> -        1, 0x1800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x7, 1, 0x1);
>  
>      dev = qdev_create(NULL, "generic_pci");
>      busdev = SYS_BUS_DEVICE(dev);
> +    ps = PCI_GEN(dev);
> +
> +    /* Set the mapping data in the device structure where:
> +     * ptr[i] = base address
> +     * ptr[i+1] = size
> +     * i = 0 => config
> +     * i = 2 => I/O
> +     * i = 4 => memory
> +     * These values are needed by the specific device code.
> +     * */
> +    hwaddr *ptr = g_malloc(6 * sizeof(hwaddr));
> +    for (i = VIRT_PCI_CFG; i <= VIRT_PCI_MEM; i++) {
> +        ptr[count++] = vbi->memmap[i].base;
> +        ptr[count++] = vbi->memmap[i].size;
> +    }
> +    ps->dt_data.fdt = vbi->fdt;
> +    ps->dt_data.irq_base = vbi->irqmap[VIRT_PCI_CFG];
> +    ps->dt_data.addr_mapping = ptr;
> +
>      qdev_init_nofail(dev);
> +
>      sysbus_mmio_map(busdev, 0, cfg_base); /* PCI config */
>      sysbus_mmio_map(busdev, 1, io_base);  /* PCI I/O */
>      sysbus_mmio_map(busdev, 2, mem_base); /* PCI memory window */
> @@ -407,8 +398,6 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
>      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
>      pci_create_simple(pci_bus, -1, "pci-ohci");
>      pci_create_simple(pci_bus, -1, "lsi53c895a");
> -
> -    g_free(nodename);
>  }
>  
>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
> @@ -454,6 +443,29 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>      return board->fdt;
>  }
>  
> +static void machvirt_finalize_dt(MachineState *machine)
> +{
> +    BusState *bus;
> +    BusChild *child;
> +    VirtBoardInfo *vbi;
> +    /* TODO store somewhere the CPU model used */
> +    vbi = find_machine_info("cortex-a15");
> +
> +    /* If the device has been successfully created, create also its
> +     * device node in the dt. */
> +    bus = sysbus_get_default();
> +    QTAILQ_FOREACH(child, &bus->children, sibling) {
> +        DeviceState *dev = child->child;
> +        if (!strcmp(object_get_typename(OBJECT(dev)), TYPE_GENERIC_PCI)) {
> +            GenericPCIClass *gpci = GENERIC_PCI_GET_CLASS(OBJECT(dev));
> +            /* create device tree node */
> +            gpci->gen_dt_node(dev);
> +        }
> +    }
> +
> +    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
> +}
> +
>  static void machvirt_init(MachineState *machine)
>  {
>      MemoryRegion *sysmem = get_system_memory();
> @@ -542,13 +554,13 @@ static void machvirt_init(MachineState *machine)
>      vbi->bootinfo.board_id = -1;
>      vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
>      vbi->bootinfo.get_dtb = machvirt_dtb;
> -    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
>  }
>  
>  static QEMUMachine machvirt_a15_machine = {
>      .name = "virt",
>      .desc = "ARM Virtual Machine",
>      .init = machvirt_init,
> +    .finalize_dt = machvirt_finalize_dt,
>      .max_cpus = 4,
>  };
>  
> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
> index 75aae45..2068079 100644
> --- a/hw/pci-host/generic-pci.c
> +++ b/hw/pci-host/generic-pci.c
> @@ -14,6 +14,7 @@
>  #include "hw/sysbus.h"
>  #include "hw/pci-host/pci_generic.h"
>  #include "exec/address-spaces.h"
> +#include "sysemu/device_tree.h"
>  
>  static const VMStateDescription pci_generic_host_vmstate = {
>      .name = "generic-host-pci",
> @@ -130,6 +131,96 @@ static void pci_generic_host_class_init(ObjectClass *klass, void *data)
>      dc->cannot_instantiate_with_device_add_yet = true;
>  }
>  
> +struct dt_irq_mapping {
> +        DeviceState *dev;
> +        uint32_t gic_phandle;
> +        int base_irq_num;
> +        uint64_t *data;
> +};
> +
> +#define IRQ_MAPPING_CELLS 14
> +/* Generate the irq_mapping data and return the number of the device attached
> + * to the device bus.
> + * */
> +static int generate_int_mapping(struct dt_irq_mapping *irq_map)
> +{
> +    BusState *inner_bus;
> +    BusChild *inner;
> +    int num_slots = 0;
> +    uint64_t *data_ptr = irq_map->data;
> +
> +    QLIST_FOREACH(inner_bus, &irq_map->dev->child_bus, sibling) {
> +        QTAILQ_FOREACH(inner, &inner_bus->children, sibling) {
> +            DeviceState *dev = inner->child;
> +            PCIDevice *pdev = PCI_DEVICE(dev);
> +            int pci_slot = PCI_SLOT(pdev->devfn);
> +
> +            uint64_t buffer[IRQ_MAPPING_CELLS] =
> +            {1, pci_slot << 11, 2, 0x00000000, 1, 0x1,
> +             1, irq_map->gic_phandle, 1, 0, 1, irq_map->base_irq_num + pci_slot,
> +             1, 0x1};

Here. I think this needs way more commenting and an explanation about how this works.

> +
> +            memcpy(data_ptr, buffer, IRQ_MAPPING_CELLS * sizeof(*buffer));
> +            num_slots++;
> +            data_ptr += IRQ_MAPPING_CELLS;
> +        }
> +    }
> +
> +    return num_slots;
> +}
> +
> +static void generate_dt_node(DeviceState *dev)
> +{
> +    PCIVPBState *s = PCI_GEN(dev);
> +    char *nodename;
> +    uint32_t gic_phandle;
> +    int num_dev;
> +    hwaddr *map = s->dt_data.addr_mapping;
> +    void *fdt = s->dt_data.fdt;
> +
> +    nodename = g_strdup_printf("/pci@%" PRIx64, map[0]);
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible",
> +                            "pci-host-cam-generic");
> +    qemu_fdt_setprop_string(fdt, nodename, "device_type", "pci");
> +    qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 0x3);
> +    qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0x2);
> +    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0x1);
> +
> +    /* config space */
> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", 2, map[0],
> +                                                            2, map[1]);
> +
> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "ranges",
> +         1, 0x01000000, 2, 0x00000000, 2, map[2], 2, map[3],
> +         1, 0x02000000, 2, 0x12000000, 2, map[4], 2, map[5]);

hard coded mach-virt addresses in generic_pci: is it really generic pci, or is it mach-virt pci?

> +
> +    gic_phandle = qemu_fdt_get_phandle(fdt, "/intc");
> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "interrupt-map-mask",
> +                                  1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);

also here some clear comment and explanation would be beneficial.

> +
> +    /* Generate the interrupt mapping according to the devices attached
> +     * to the PCI bus of the device. */
> +    uint64_t *int_mapping_data = g_malloc0(IRQ_MAPPING_CELLS * sizeof(uint64_t)
> +                                                            * MAX_PCI_DEVICES);
> +
> +    struct dt_irq_mapping dt_map = {
> +        .dev = dev,
> +        .gic_phandle = gic_phandle,
> +        .base_irq_num = s->dt_data.irq_base,
> +        .data = int_mapping_data
> +    };
> +
> +    num_dev = generate_int_mapping(&dt_map);
> +    qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "interrupt-map",
> +                        (num_dev * IRQ_MAPPING_CELLS)/2, int_mapping_data);
> +
> +    g_free(int_mapping_data);
> +    g_free(nodename);
> +    /* Once the dt node is created, this data is no longer necessary */
> +    g_free(s->dt_data.addr_mapping);
> +}
> +
>  static const TypeInfo pci_generic_host_info = {
>      .name          = TYPE_GENERIC_PCI_HOST,
>      .parent        = TYPE_PCI_DEVICE,
> @@ -140,9 +231,11 @@ static const TypeInfo pci_generic_host_info = {
>  static void pci_generic_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    GenericPCIClass *pc = GENERIC_PCI_CLASS(klass);
>  
>      dc->realize = pci_generic_host_realize;
>      dc->vmsd = &pci_generic_host_vmstate;
> +    pc->gen_dt_node = generate_dt_node;
>  }
>  
>  static const TypeInfo pci_generic_info = {
> @@ -151,6 +244,7 @@ static const TypeInfo pci_generic_info = {
>      .instance_size = sizeof(PCIVPBState),
>      .instance_init = pci_generic_host_init,
>      .class_init    = pci_generic_class_init,
> +    .class_size    = sizeof(GenericPCIClass),
>  };
>  
>  static void generic_pci_host_register_types(void)
> diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
> index 2367d80..039549f 100644
> --- a/include/hw/pci-host/pci_generic.h
> +++ b/include/hw/pci-host/pci_generic.h
> @@ -7,6 +7,12 @@
>  
>  #define MAX_PCI_DEVICES 10
>  
> +struct dt_data {
> +    void *fdt;
> +    int irq_base;
> +    hwaddr *addr_mapping;
> +};
> +
>  typedef struct {
>      PCIHostState parent_obj;
>  
> @@ -22,8 +28,17 @@ typedef struct {
>      MemoryRegion pci_mem_window;
>      PCIBus pci_bus;
>      PCIDevice pci_dev;
> +    /* Device tree data set by the machine
> +     */
> +    struct dt_data dt_data;
>  } PCIVPBState;
>  
> +typedef struct GenericPCIClass {
> +    PCIDeviceClass parent_class;
> +
> +    void (*gen_dt_node)(DeviceState *dev);
> +} GenericPCIClass;
> +
>  #define TYPE_GENERIC_PCI "generic_pci"
>  #define PCI_GEN(obj) \
>      OBJECT_CHECK(PCIVPBState, (obj), TYPE_GENERIC_PCI)
> @@ -32,4 +47,9 @@ typedef struct {
>  #define PCI_GEN_HOST(obj) \
>      OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
>  
> +#define GENERIC_PCI_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(GenericPCIClass, (klass), TYPE_GENERIC_PCI)
> +#define GENERIC_PCI_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(GenericPCIClass, (obj), TYPE_GENERIC_PCI)
> +
>  #endif
> 

Re: [Qemu-devel] [RFC PATCH 6/8] generic_pci: generate dt node after devices init

[alvise rigo]

On Wed, Nov 5, 2014 at 1:26 PM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> On 11.07.2014 09:21, Alvise Rigo wrote:
>> Keeping advantage of the finalize_dt QEMUMachine function, the mach-virt
>> machine now completes the device tree creation after that all the
>> generic devices have been instantiated. This allows to generate the
>> interrupt-map node according to the devices attached to the PCI bus.
>> These devices can be specified either as command line argument (like
>> -device lsi53c895a addr=0x5 ...) or explicitly inside the machine
>> definition with the pci_create_simple method.
>>
>> Fill also the generic_pci state with the offsets and sizes of the memory
>> regions needed by the PCI system. The offset in the machine address
>> space of these regions (config, IO and memory) is specified by the
>> mach-virt platform.
>
> If this is mach-virt specific, why is this called generic_pci..?
> Is it generic ARM pci?

The overall implementation shouldn't be mach-virt specific, but at the
moment I don't see any other use case for it.
Maybe in the future this controller could come in handy to support
other ARM platforms with PCI support.

>
>>
>> TODO:
>> - Part of the ranges device node is still hardcoded.
>
> Also this would benefit from extensive commenting.
> Especially the exact meaning of the interrupt mapping, and if it is ARM specific,
> how the interrupts end up being mapped to gic irqs.
> [see below]
>
>>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>  hw/arm/virt.c                     | 80 +++++++++++++++++++--------------
>>  hw/pci-host/generic-pci.c         | 94 +++++++++++++++++++++++++++++++++++++++
>>  include/hw/pci-host/pci_generic.h | 20 +++++++++
>>  3 files changed, 160 insertions(+), 34 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index a433902..e182282 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -358,44 +358,35 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
>>      PCIBus *pci_bus;
>>      DeviceState *dev;
>>      SysBusDevice *busdev;
>> -    uint32_t gic_phandle;
>> -    char *nodename;
>> -    int i;
>> +    PCIVPBState *ps;
>> +    int i, count = 0;
>>      hwaddr cfg_base = vbi->memmap[VIRT_PCI_CFG].base;
>> -    hwaddr cfg_size = vbi->memmap[VIRT_PCI_CFG].size;
>>      hwaddr io_base = vbi->memmap[VIRT_PCI_IO].base;
>> -    hwaddr io_size = vbi->memmap[VIRT_PCI_IO].size;
>>      hwaddr mem_base = vbi->memmap[VIRT_PCI_MEM].base;
>> -    hwaddr mem_size = vbi->memmap[VIRT_PCI_MEM].size;
>> -
>> -    nodename = g_strdup_printf("/pci@%" PRIx64, 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);
>> -
>> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, cfg_base,
>> -                                                           2, cfg_size);
>> -
>> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
>> -        1, 0x01000000, 2, 0x00000000, 2, io_base, 2, io_size,
>> -        1, 0x02000000, 2, 0x12000000, 2, mem_size, 2, mem_size);
>> -
>> -    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
>> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
>> -        1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);
>> -    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map",
>> -        1, 0x0000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x4, 1, 0x1,
>> -        1, 0x0800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x5, 1, 0x1,
>> -        1, 0x1000, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x6, 1, 0x1,
>> -        1, 0x1800, 2, 0x00000000, 1, 0x1, 1, gic_phandle, 1, 0, 1, 0x7, 1, 0x1);
>>
>>      dev = qdev_create(NULL, "generic_pci");
>>      busdev = SYS_BUS_DEVICE(dev);
>> +    ps = PCI_GEN(dev);
>> +
>> +    /* Set the mapping data in the device structure where:
>> +     * ptr[i] = base address
>> +     * ptr[i+1] = size
>> +     * i = 0 => config
>> +     * i = 2 => I/O
>> +     * i = 4 => memory
>> +     * These values are needed by the specific device code.
>> +     * */
>> +    hwaddr *ptr = g_malloc(6 * sizeof(hwaddr));
>> +    for (i = VIRT_PCI_CFG; i <= VIRT_PCI_MEM; i++) {
>> +        ptr[count++] = vbi->memmap[i].base;
>> +        ptr[count++] = vbi->memmap[i].size;
>> +    }
>> +    ps->dt_data.fdt = vbi->fdt;
>> +    ps->dt_data.irq_base = vbi->irqmap[VIRT_PCI_CFG];
>> +    ps->dt_data.addr_mapping = ptr;
>> +
>>      qdev_init_nofail(dev);
>> +
>>      sysbus_mmio_map(busdev, 0, cfg_base); /* PCI config */
>>      sysbus_mmio_map(busdev, 1, io_base);  /* PCI I/O */
>>      sysbus_mmio_map(busdev, 2, mem_base); /* PCI memory window */
>> @@ -407,8 +398,6 @@ static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
>>      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
>>      pci_create_simple(pci_bus, -1, "pci-ohci");
>>      pci_create_simple(pci_bus, -1, "lsi53c895a");
>> -
>> -    g_free(nodename);
>>  }
>>
>>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>> @@ -454,6 +443,29 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>>      return board->fdt;
>>  }
>>
>> +static void machvirt_finalize_dt(MachineState *machine)
>> +{
>> +    BusState *bus;
>> +    BusChild *child;
>> +    VirtBoardInfo *vbi;
>> +    /* TODO store somewhere the CPU model used */
>> +    vbi = find_machine_info("cortex-a15");
>> +
>> +    /* If the device has been successfully created, create also its
>> +     * device node in the dt. */
>> +    bus = sysbus_get_default();
>> +    QTAILQ_FOREACH(child, &bus->children, sibling) {
>> +        DeviceState *dev = child->child;
>> +        if (!strcmp(object_get_typename(OBJECT(dev)), TYPE_GENERIC_PCI)) {
>> +            GenericPCIClass *gpci = GENERIC_PCI_GET_CLASS(OBJECT(dev));
>> +            /* create device tree node */
>> +            gpci->gen_dt_node(dev);
>> +        }
>> +    }
>> +
>> +    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
>> +}
>> +
>>  static void machvirt_init(MachineState *machine)
>>  {
>>      MemoryRegion *sysmem = get_system_memory();
>> @@ -542,13 +554,13 @@ static void machvirt_init(MachineState *machine)
>>      vbi->bootinfo.board_id = -1;
>>      vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
>>      vbi->bootinfo.get_dtb = machvirt_dtb;
>> -    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
>>  }
>>
>>  static QEMUMachine machvirt_a15_machine = {
>>      .name = "virt",
>>      .desc = "ARM Virtual Machine",
>>      .init = machvirt_init,
>> +    .finalize_dt = machvirt_finalize_dt,
>>      .max_cpus = 4,
>>  };
>>
>> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
>> index 75aae45..2068079 100644
>> --- a/hw/pci-host/generic-pci.c
>> +++ b/hw/pci-host/generic-pci.c
>> @@ -14,6 +14,7 @@
>>  #include "hw/sysbus.h"
>>  #include "hw/pci-host/pci_generic.h"
>>  #include "exec/address-spaces.h"
>> +#include "sysemu/device_tree.h"
>>
>>  static const VMStateDescription pci_generic_host_vmstate = {
>>      .name = "generic-host-pci",
>> @@ -130,6 +131,96 @@ static void pci_generic_host_class_init(ObjectClass *klass, void *data)
>>      dc->cannot_instantiate_with_device_add_yet = true;
>>  }
>>
>> +struct dt_irq_mapping {
>> +        DeviceState *dev;
>> +        uint32_t gic_phandle;
>> +        int base_irq_num;
>> +        uint64_t *data;
>> +};
>> +
>> +#define IRQ_MAPPING_CELLS 14
>> +/* Generate the irq_mapping data and return the number of the device attached
>> + * to the device bus.
>> + * */
>> +static int generate_int_mapping(struct dt_irq_mapping *irq_map)
>> +{
>> +    BusState *inner_bus;
>> +    BusChild *inner;
>> +    int num_slots = 0;
>> +    uint64_t *data_ptr = irq_map->data;
>> +
>> +    QLIST_FOREACH(inner_bus, &irq_map->dev->child_bus, sibling) {
>> +        QTAILQ_FOREACH(inner, &inner_bus->children, sibling) {
>> +            DeviceState *dev = inner->child;
>> +            PCIDevice *pdev = PCI_DEVICE(dev);
>> +            int pci_slot = PCI_SLOT(pdev->devfn);
>> +
>> +            uint64_t buffer[IRQ_MAPPING_CELLS] =
>> +            {1, pci_slot << 11, 2, 0x00000000, 1, 0x1,
>> +             1, irq_map->gic_phandle, 1, 0, 1, irq_map->base_irq_num + pci_slot,
>> +             1, 0x1};
>
> Here. I think this needs way more commenting and an explanation about how this works.

Yes, indeed. This interrupts mapping node is defined here:
http://www.unixag-kl.fh-kl.de/~jkunz/tmp/imap0_9d.pdf

The purpose of this mapping, in this specific case, is to map
interrupts from the PCI domain to the interrupt controller domain (the
GIC in our case).
This code produces a node like:
interrupt-map = <0x1800 0x0 0x0 0x1 0x8001 0x0 0x2f 0x1, ...>
where, for each tuple, the first four values specify the interrupt
according to the PCI domain: "0x1800 0x0 0x0" is the device's unit
address (PCI device slot number) while "0x1" is the PCI interrupt
number.
The fifth value (0x8001) represent the phandle of the interrupt
controller (the node that somehow defines the new domain).
The last three values define the interrupt in the new domain, and
follow the exact format required by the interrupt controller.
For sake of completeness, the GIC node is reported below (note phandle
address and #interrupt-cells).
intc {
        phandle = <0x8001>;
        reg = <0x0 0x8000000 0x0 0x10000 0x0 0x8010000 0x0 0x10000>;
        interrupt-controller;
        #interrupt-cells = <0x3>;
        compatible = "arm,cortex-a15-gic";
};

>
>> +
>> +            memcpy(data_ptr, buffer, IRQ_MAPPING_CELLS * sizeof(*buffer));
>> +            num_slots++;
>> +            data_ptr += IRQ_MAPPING_CELLS;
>> +        }
>> +    }
>> +
>> +    return num_slots;
>> +}
>> +
>> +static void generate_dt_node(DeviceState *dev)
>> +{
>> +    PCIVPBState *s = PCI_GEN(dev);
>> +    char *nodename;
>> +    uint32_t gic_phandle;
>> +    int num_dev;
>> +    hwaddr *map = s->dt_data.addr_mapping;
>> +    void *fdt = s->dt_data.fdt;
>> +
>> +    nodename = g_strdup_printf("/pci@%" PRIx64, map[0]);
>> +    qemu_fdt_add_subnode(fdt, nodename);
>> +    qemu_fdt_setprop_string(fdt, nodename, "compatible",
>> +                            "pci-host-cam-generic");
>> +    qemu_fdt_setprop_string(fdt, nodename, "device_type", "pci");
>> +    qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 0x3);
>> +    qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0x2);
>> +    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0x1);
>> +
>> +    /* config space */
>> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", 2, map[0],
>> +                                                            2, map[1]);
>> +
>> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "ranges",
>> +         1, 0x01000000, 2, 0x00000000, 2, map[2], 2, map[3],
>> +         1, 0x02000000, 2, 0x12000000, 2, map[4], 2, map[5]);
>
> hard coded mach-virt addresses in generic_pci: is it really generic pci, or is it mach-virt pci?

The hardcoded values you see are related to the generic-pci device,
while the values passed inside the map array define how to map the
device regions to memory.
These values should be moved in the header file, since I don't expect
them to change.

>
>> +
>> +    gic_phandle = qemu_fdt_get_phandle(fdt, "/intc");
>> +    qemu_fdt_setprop_sized_cells(fdt, nodename, "interrupt-map-mask",
>> +                                  1, 0xf800, 1, 0x0, 1, 0x0, 1, 0x7);
>
> also here some clear comment and explanation would be beneficial.

The interrupt map property tells how to perform the lookup in the
interrupt-map table.
This mask is useful when not all the bits of a unit interrupt
specifier are relevant for the lookup. In the PCI case the value
0xf800 0x0 0x0 0x7 will keep only PCI slot number and interrupt number
(which in basically always 1).

Regards,
alvise

>
>> +
>> +    /* Generate the interrupt mapping according to the devices attached
>> +     * to the PCI bus of the device. */
>> +    uint64_t *int_mapping_data = g_malloc0(IRQ_MAPPING_CELLS * sizeof(uint64_t)
>> +                                                            * MAX_PCI_DEVICES);
>> +
>> +    struct dt_irq_mapping dt_map = {
>> +        .dev = dev,
>> +        .gic_phandle = gic_phandle,
>> +        .base_irq_num = s->dt_data.irq_base,
>> +        .data = int_mapping_data
>> +    };
>> +
>> +    num_dev = generate_int_mapping(&dt_map);
>> +    qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "interrupt-map",
>> +                        (num_dev * IRQ_MAPPING_CELLS)/2, int_mapping_data);
>> +
>> +    g_free(int_mapping_data);
>> +    g_free(nodename);
>> +    /* Once the dt node is created, this data is no longer necessary */
>> +    g_free(s->dt_data.addr_mapping);
>> +}
>> +
>>  static const TypeInfo pci_generic_host_info = {
>>      .name          = TYPE_GENERIC_PCI_HOST,
>>      .parent        = TYPE_PCI_DEVICE,
>> @@ -140,9 +231,11 @@ static const TypeInfo pci_generic_host_info = {
>>  static void pci_generic_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> +    GenericPCIClass *pc = GENERIC_PCI_CLASS(klass);
>>
>>      dc->realize = pci_generic_host_realize;
>>      dc->vmsd = &pci_generic_host_vmstate;
>> +    pc->gen_dt_node = generate_dt_node;
>>  }
>>
>>  static const TypeInfo pci_generic_info = {
>> @@ -151,6 +244,7 @@ static const TypeInfo pci_generic_info = {
>>      .instance_size = sizeof(PCIVPBState),
>>      .instance_init = pci_generic_host_init,
>>      .class_init    = pci_generic_class_init,
>> +    .class_size    = sizeof(GenericPCIClass),
>>  };
>>
>>  static void generic_pci_host_register_types(void)
>> diff --git a/include/hw/pci-host/pci_generic.h b/include/hw/pci-host/pci_generic.h
>> index 2367d80..039549f 100644
>> --- a/include/hw/pci-host/pci_generic.h
>> +++ b/include/hw/pci-host/pci_generic.h
>> @@ -7,6 +7,12 @@
>>
>>  #define MAX_PCI_DEVICES 10
>>
>> +struct dt_data {
>> +    void *fdt;
>> +    int irq_base;
>> +    hwaddr *addr_mapping;
>> +};
>> +
>>  typedef struct {
>>      PCIHostState parent_obj;
>>
>> @@ -22,8 +28,17 @@ typedef struct {
>>      MemoryRegion pci_mem_window;
>>      PCIBus pci_bus;
>>      PCIDevice pci_dev;
>> +    /* Device tree data set by the machine
>> +     */
>> +    struct dt_data dt_data;
>>  } PCIVPBState;
>>
>> +typedef struct GenericPCIClass {
>> +    PCIDeviceClass parent_class;
>> +
>> +    void (*gen_dt_node)(DeviceState *dev);
>> +} GenericPCIClass;
>> +
>>  #define TYPE_GENERIC_PCI "generic_pci"
>>  #define PCI_GEN(obj) \
>>      OBJECT_CHECK(PCIVPBState, (obj), TYPE_GENERIC_PCI)
>> @@ -32,4 +47,9 @@ typedef struct {
>>  #define PCI_GEN_HOST(obj) \
>>      OBJECT_CHECK(PCIDevice, (obj), TYPE_GENERIC_PCIHOST)
>>
>> +#define GENERIC_PCI_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(GenericPCIClass, (klass), TYPE_GENERIC_PCI)
>> +#define GENERIC_PCI_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(GenericPCIClass, (obj), TYPE_GENERIC_PCI)
>> +
>>  #endif
>>
>
>
>
>

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

[Claudio Fontana]

Hi Alvise,

I now got to test the series for my use case, in particular to enable the
ARM 64bit OSv guest (OSv's devices come from pci + virtio).

Could you respin the series, possibly including also Rob's patches,
addressing the issues which have been raised before?

Thanks!

Claudio


On 11.07.2014 09:21, Alvise Rigo wrote:
> This patch series is based on the previous work [1] and [2] by Rob
> Herring and it tries to enhance this work on these points:
> 
> - Some of the hardcoded values have been moved to an header file.  This
>   header file is also used to share some device structures with the
>   mach-virt machine.
> - The interrupt-map dt node generation has been revisited; it is now
>   done after the generic devices init so that it's possible to attach
>   PCI devices by mean of the qdev infrastructure. This allows to have
>   several devices in the PCI bus, with the current limitation of one
>   interrupt per PCI slot.
> 
> Probably the most objectionable part of these patches regards the way
> some data and definitions have been shared between the machine and the
> device code; a better solution is still under evaluation. Any advice on
> this and on the rest of the work is highly appreciated.
> 
> This work has been tested attaching several PCI devices to the mach-virt
> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
> lsi53c895a and pci-ohci (all attached at the same time).
> Even if the original work was not changed in its core functionalities, I
> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
> After attaching several qcow2 images, formatting and filling them, I
> didn't notice anything wrong. Am I missing something?
> 
> 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
> 
> Alvise Rigo (8):
>   mach-virt: move GIC inside mach-virt structure
>   mach-virt: improve PCI memory topology definition
>   QEMUMachine: finalize_dt function
>   generic_pci: create header file
>   generic_pci: create own map irq function
>   generic_pci: generate dt node after devices init
>   generic_pci: realize device with machine data
>   generic_pci: add interrupt map structures
> 
>  hw/arm/virt.c                     | 110 +++++++++++++++---------
>  hw/pci-host/generic-pci.c         | 173 +++++++++++++++++++++++++++++---------
>  include/hw/boards.h               |   4 +
>  include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
>  vl.c                              |   5 ++
>  5 files changed, 277 insertions(+), 81 deletions(-)
>  create mode 100644 include/hw/pci-host/pci_generic.h
> 

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

[alvise rigo]

Hi Claudio,

On Fri, Nov 7, 2014 at 4:40 PM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
>
> Hi Alvise,
>
> I now got to test the series for my use case, in particular to enable the
> ARM 64bit OSv guest (OSv's devices come from pci + virtio).
>
> Could you respin the series, possibly including also Rob's patches,
> addressing the issues which have been raised before?

Yes, I hope to have something for the next week.

Regards,
alvise

>
>
> Thanks!
>
> Claudio
>
>
> On 11.07.2014 09:21, Alvise Rigo wrote:
> > This patch series is based on the previous work [1] and [2] by Rob
> > Herring and it tries to enhance this work on these points:
> >
> > - Some of the hardcoded values have been moved to an header file.  This
> >   header file is also used to share some device structures with the
> >   mach-virt machine.
> > - The interrupt-map dt node generation has been revisited; it is now
> >   done after the generic devices init so that it's possible to attach
> >   PCI devices by mean of the qdev infrastructure. This allows to have
> >   several devices in the PCI bus, with the current limitation of one
> >   interrupt per PCI slot.
> >
> > Probably the most objectionable part of these patches regards the way
> > some data and definitions have been shared between the machine and the
> > device code; a better solution is still under evaluation. Any advice on
> > this and on the rest of the work is highly appreciated.
> >
> > This work has been tested attaching several PCI devices to the mach-virt
> > platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
> > lsi53c895a and pci-ohci (all attached at the same time).
> > Even if the original work was not changed in its core functionalities, I
> > couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
> > After attaching several qcow2 images, formatting and filling them, I
> > didn't notice anything wrong. Am I missing something?
> >
> > 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
> >
> > Alvise Rigo (8):
> >   mach-virt: move GIC inside mach-virt structure
> >   mach-virt: improve PCI memory topology definition
> >   QEMUMachine: finalize_dt function
> >   generic_pci: create header file
> >   generic_pci: create own map irq function
> >   generic_pci: generate dt node after devices init
> >   generic_pci: realize device with machine data
> >   generic_pci: add interrupt map structures
> >
> >  hw/arm/virt.c                     | 110 +++++++++++++++---------
> >  hw/pci-host/generic-pci.c         | 173 +++++++++++++++++++++++++++++---------
> >  include/hw/boards.h               |   4 +
> >  include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
> >  vl.c                              |   5 ++
> >  5 files changed, 277 insertions(+), 81 deletions(-)
> >  create mode 100644 include/hw/pci-host/pci_generic.h
> >
>
>

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

[Ming Lei]

On Mon, Nov 10, 2014 at 6:00 PM, alvise rigo
<a.rigo@virtualopensystems.com> wrote:
> Hi Claudio,
>
> On Fri, Nov 7, 2014 at 4:40 PM, Claudio Fontana
> <claudio.fontana@huawei.com> wrote:
>>
>> Hi Alvise,
>>
>> I now got to test the series for my use case, in particular to enable the
>> ARM 64bit OSv guest (OSv's devices come from pci + virtio).
>>
>> Could you respin the series, possibly including also Rob's patches,
>> addressing the issues which have been raised before?
>
> Yes, I hope to have something for the next week.

I have tested this serials with Rob's two patches, and it works fine
on ARMv7 VM, but ARMv8 VM can't boot.  'git bisect' told
me the 1st bad patch is below one:

      generic_pci: generate dt node after devices init


Thanks,
-- 
Ming Lei

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

[Ming Lei]

On Tue, Nov 11, 2014 at 11:24 AM, Ming Lei <tom.leiming@gmail.com> wrote:
> On Mon, Nov 10, 2014 at 6:00 PM, alvise rigo
> <a.rigo@virtualopensystems.com> wrote:
>> Hi Claudio,
>>
>> On Fri, Nov 7, 2014 at 4:40 PM, Claudio Fontana
>> <claudio.fontana@huawei.com> wrote:
>>>
>>> Hi Alvise,
>>>
>>> I now got to test the series for my use case, in particular to enable the
>>> ARM 64bit OSv guest (OSv's devices come from pci + virtio).
>>>
>>> Could you respin the series, possibly including also Rob's patches,
>>> addressing the issues which have been raised before?
>>
>> Yes, I hope to have something for the next week.
>
> I have tested this serials with Rob's two patches, and it works fine
> on ARMv7 VM, but ARMv8 VM can't boot.  'git bisect' told
> me the 1st bad patch is below one:
>
>       generic_pci: generate dt node after devices init

That is because you didn't support 'cortex-a57' and 'host',
so please add that in your next version.

Thanks
Ming Lei

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

[Claudio Fontana]

On 11.11.2014 05:22, Ming Lei wrote:
> On Tue, Nov 11, 2014 at 11:24 AM, Ming Lei <tom.leiming@gmail.com> wrote:
>> On Mon, Nov 10, 2014 at 6:00 PM, alvise rigo
>> <a.rigo@virtualopensystems.com> wrote:
>>> Hi Claudio,
>>>
>>> On Fri, Nov 7, 2014 at 4:40 PM, Claudio Fontana
>>> <claudio.fontana@huawei.com> wrote:
>>>>
>>>> Hi Alvise,
>>>>
>>>> I now got to test the series for my use case, in particular to enable the
>>>> ARM 64bit OSv guest (OSv's devices come from pci + virtio).
>>>>
>>>> Could you respin the series, possibly including also Rob's patches,
>>>> addressing the issues which have been raised before?
>>>
>>> Yes, I hope to have something for the next week.
>>
>> I have tested this serials with Rob's two patches, and it works fine
>> on ARMv7 VM, but ARMv8 VM can't boot.  'git bisect' told
>> me the 1st bad patch is below one:
>>
>>       generic_pci: generate dt node after devices init
> 
> That is because you didn't support 'cortex-a57' and 'host',
> so please add that in your next version.
> 
> Thanks
> Ming Lei
> 

Hi,

I noticed that as well, but it actually somehow works for me on AArch64 with a kvm guest (OSv),
after some fighting (there is a QEMU issue that could be addressed in fact but it's OT).

However by all means do add explicit cortex-a57 and host support as needed..

Ciao,

Claudio