[Qemu-devel] [PATCH v4 0/4] Add support for for GICv2m and MSIs to arm-virt

[Qemu-devel] [PATCH v4 0/4] Add support for for GICv2m and MSIs to arm-virt

[Christoffer Dall]

Now when we have a host generic PCIe controller in the virt board, it
would be nice to be able to use MSIs so that we can eventually enable
VHOST with KVM.

With these patches you can use MSIs with TCG and with KVM, but you still
need some fixes for the mapping of the IRQ index to the GSI number for
IRQFD to work.  A separate series that enables IRQFD and vhost
is available: "ARM adaptations for vhost irqfd setup"
https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01054.html)

Tested with KVM on XGene and with TCG by configuring a virtio-pci
network adapter for the guest and verifying MSIs going through as
expected.

Rebased on target-arm.next, see the individual patches for detailed
changelogs.

Christoffer Dall (4):
  target-arm: Add GIC phandle to VirtBoardInfo
  arm_gicv2m: Add GICv2m widget to support MSIs
  target-arm: Extend the gic node properties
  target-arm: Add the GICv2m to the virt board

 hw/arm/virt.c         |  73 ++++++++++++++-----
 hw/intc/Makefile.objs |   1 +
 hw/intc/arm_gicv2m.c  | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |   2 +
 4 files changed, 248 insertions(+), 18 deletions(-)
 create mode 100644 hw/intc/arm_gicv2m.c

-- 
2.1.2.330.g565301e.dirty

[Qemu-devel] [PATCH v4 1/4] target-arm: Add GIC phandle to VirtBoardInfo

[Christoffer Dall]

Instead of passing the GIC phandle around between functions, add it to
the VirtBoardInfo just like we do for the clock_phandle.  We are about
to add the v2m phandle as well, and it's easier not having to pass
around a bunch of phandles, return multiple values from functions, etc.

Reviewed-by: Eric Auger <eric.auger@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
Changes since v3:
 - Added reviewed-by tag
Changes since v2:
 - None
Changes since v1:
 - Added reviewed-by tag

 hw/arm/virt.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 05db8cb..e5235ef 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -69,6 +69,7 @@ typedef struct VirtBoardInfo {
     void *fdt;
     int fdt_size;
     uint32_t clock_phandle;
+    uint32_t gic_phandle;
 } VirtBoardInfo;
 
 typedef struct {
@@ -299,12 +300,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
     }
 }
 
-static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
+static void fdt_add_gic_node(VirtBoardInfo *vbi)
 {
-    uint32_t gic_phandle;
 
-    gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
-    qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", gic_phandle);
+    vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+    qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
 
     qemu_fdt_add_subnode(vbi->fdt, "/intc");
     /* 'cortex-a15-gic' means 'GIC v2' */
@@ -317,12 +317,10 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
                                      2, vbi->memmap[VIRT_GIC_DIST].size,
                                      2, vbi->memmap[VIRT_GIC_CPU].base,
                                      2, vbi->memmap[VIRT_GIC_CPU].size);
-    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
-
-    return gic_phandle;
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
-static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
 {
     /* We create a standalone GIC v2 */
     DeviceState *gicdev;
@@ -371,7 +369,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
         pic[i] = qdev_get_gpio_in(gicdev, i);
     }
 
-    return fdt_add_gic_node(vbi);
+    fdt_add_gic_node(vbi);
 }
 
 static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -618,8 +616,7 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
                            0x7           /* PCI irq */);
 }
 
-static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
-                        uint32_t gic_phandle)
+static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     hwaddr base_mmio = vbi->memmap[VIRT_PCIE_MMIO].base;
     hwaddr size_mmio = vbi->memmap[VIRT_PCIE_MMIO].size;
@@ -685,7 +682,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
                                  2, base_mmio, 2, size_mmio);
 
     qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
-    create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
+    create_pcie_irq_map(vbi, vbi->gic_phandle, irq, nodename);
 
     g_free(nodename);
 }
@@ -717,7 +714,6 @@ static void machvirt_init(MachineState *machine)
     VirtBoardInfo *vbi;
     VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state);
     VirtGuestInfo *guest_info = &guest_info_state->info;
-    uint32_t gic_phandle;
     char **cpustr;
 
     if (!cpu_model) {
@@ -794,13 +790,13 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vbi);
 
-    gic_phandle = create_gic(vbi, pic);
+    create_gic(vbi, pic);
 
     create_uart(vbi, pic);
 
     create_rtc(vbi, pic);
 
-    create_pcie(vbi, pic, gic_phandle);
+    create_pcie(vbi, pic);
 
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
-- 
2.1.2.330.g565301e.dirty

[Qemu-devel] [PATCH v4 2/4] arm_gicv2m: Add GICv2m widget to support MSIs

[Christoffer Dall]

The ARM GICv2m widget is a little device that handles MSI interrupt
writes to a trigger register and ties them to a range of interrupt lines
wires to the GIC.  It has a few status/id registers and the interrupt wires,
and that's about it.

A board instantiates the device by setting the base SPI number and
number SPIs for the frame.  The base-spi parameter is indexed in the SPI
number space only, so base-spi == 0, means IRQ number 32.  When a device
(the PCI host controller) writes to the trigger register, the payload is
the GIC IRQ number, so we have to subtract 32 from that and then index
into our frame of SPIs.

When instantiating a GICv2m device, tell PCI that we have instantiated
something that can deal with MSIs.  We rely on the board actually wiring
up the GICv2m to the PCI host controller.

Reviewed-by: Eric Auger <eric.auger@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
Changes since v3:
 - Added reviewed-by tag
Changes since v2:
 - Renamed QOM type to "arm-gicv2m"
Changes since v1:
 - Check that writes to MSI_SETSPI are within the lower boundary as well
 - Move gicv2m to common-obj in Makefile
 - Separate switch case and comment for impdef regs
 - Clearly document what is emulated
 - Allow 16 bit lower accesses to MSI_SETSPI regs
 - Fix commit grammar error
 - Remove stray pixman commit

 hw/intc/Makefile.objs |   1 +
 hw/intc/arm_gicv2m.c  | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 191 insertions(+)
 create mode 100644 hw/intc/arm_gicv2m.c

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 843864a..092d8a8 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(CONFIG_SLAVIO) += slavio_intctl.o
 common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c
new file mode 100644
index 0000000..9f84f72
--- /dev/null
+++ b/hw/intc/arm_gicv2m.c
@@ -0,0 +1,190 @@
+/*
+ *  GICv2m extension for MSI/MSI-x support with a GICv2-based system
+ *
+ * Copyright (C) 2015 Linaro, All rights reserved.
+ *
+ * Author: Christoffer Dall <christoffer.dall@linaro.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* This file implements an emulated GICv2m widget as described in the ARM
+ * Server Base System Architecture (SBSA) specification Version 2.2
+ * (ARM-DEN-0029 v2.2) pages 35-39 without any optional implementation defined
+ * identification registers and with a single non-secure MSI register frame.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/pci/msi.h"
+
+#define TYPE_ARM_GICV2M "arm-gicv2m"
+#define ARM_GICV2M(obj) OBJECT_CHECK(ARMGICv2mState, (obj), TYPE_ARM_GICV2M)
+
+#define GICV2M_NUM_SPI_MAX 128
+
+#define V2M_MSI_TYPER           0x008
+#define V2M_MSI_SETSPI_NS       0x040
+#define V2M_MSI_IIDR            0xFCC
+#define V2M_IIDR0               0xFD0
+#define V2M_IIDR11              0xFFC
+
+#define PRODUCT_ID_QEMU         0x51 /* ASCII code Q */
+
+typedef struct ARMGICv2mState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    qemu_irq spi[GICV2M_NUM_SPI_MAX];
+
+    uint32_t base_spi;
+    uint32_t num_spi;
+} ARMGICv2mState;
+
+static void gicv2m_set_irq(void *opaque, int irq)
+{
+    ARMGICv2mState *s = (ARMGICv2mState *)opaque;
+
+    qemu_irq_pulse(s->spi[irq]);
+}
+
+static uint64_t gicv2m_read(void *opaque, hwaddr offset,
+                            unsigned size)
+{
+    ARMGICv2mState *s = (ARMGICv2mState *)opaque;
+    uint32_t val;
+
+    if (size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_read: bad size %u\n", size);
+        return 0;
+    }
+
+    switch (offset) {
+    case V2M_MSI_TYPER:
+        val = (s->base_spi + 32) << 16;
+        val |= s->num_spi;
+        return val;
+    case V2M_MSI_IIDR:
+        /* We don't have any valid implementor so we leave that field as zero
+         * and we return 0 in the arch revision as per the spec.
+         */
+        return (PRODUCT_ID_QEMU << 20);
+    case V2M_IIDR0 ... V2M_IIDR11:
+        /* We do not implement any optional identification registers and the
+         * mandatory MSI_PIDR2 register reads as 0x0, so we capture all
+         * implementation defined registers here.
+         */
+        return 0;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "gicv2m_read: Bad offset %x\n", (int)offset);
+        return 0;
+    }
+}
+
+static void gicv2m_write(void *opaque, hwaddr offset,
+                        uint64_t value, unsigned size)
+{
+    ARMGICv2mState *s = (ARMGICv2mState *)opaque;
+
+    if (size != 2 && size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_write: bad size %u\n", size);
+        return;
+    }
+
+    switch (offset) {
+    case V2M_MSI_SETSPI_NS: {
+        int spi;
+
+        spi = (value & 0x3ff) - (s->base_spi + 32);
+        if (spi >= 0 && spi < s->num_spi) {
+            gicv2m_set_irq(s, spi);
+        }
+        return;
+    }
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "gicv2m_write: Bad offset %x\n", (int)offset);
+    }
+}
+
+static const MemoryRegionOps gicv2m_ops = {
+    .read = gicv2m_read,
+    .write = gicv2m_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void gicv2m_realize(DeviceState *dev, Error **errp)
+{
+    ARMGICv2mState *s = ARM_GICV2M(dev);
+    int i;
+
+    if (s->num_spi > GICV2M_NUM_SPI_MAX) {
+        error_setg(errp,
+                   "requested %u SPIs exceeds GICv2m frame maximum %d",
+                   s->num_spi, GICV2M_NUM_SPI_MAX);
+        return;
+    }
+
+    if (s->base_spi + 32 > 1020 - s->num_spi) {
+        error_setg(errp,
+                   "requested base SPI %u+%u exceeds max. number 1020",
+                   s->base_spi + 32, s->num_spi);
+        return;
+    }
+
+    for (i = 0; i < s->num_spi; i++) {
+        sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->spi[i]);
+    }
+
+    msi_supported = true;
+}
+
+static void gicv2m_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    ARMGICv2mState *s = ARM_GICV2M(obj);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &gicv2m_ops, s,
+                          "gicv2m", 0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static Property gicv2m_properties[] = {
+    DEFINE_PROP_UINT32("base-spi", ARMGICv2mState, base_spi, 0),
+    DEFINE_PROP_UINT32("num-spi", ARMGICv2mState, num_spi, 64),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void gicv2m_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = gicv2m_properties;
+    dc->realize = gicv2m_realize;
+}
+
+static const TypeInfo gicv2m_info = {
+    .name          = TYPE_ARM_GICV2M,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ARMGICv2mState),
+    .instance_init = gicv2m_init,
+    .class_init    = gicv2m_class_init,
+};
+
+static void gicv2m_register_types(void)
+{
+    type_register_static(&gicv2m_info);
+}
+
+type_init(gicv2m_register_types)
-- 
2.1.2.330.g565301e.dirty

[Qemu-devel] [PATCH v4 3/4] target-arm: Extend the gic node properties

[Christoffer Dall]

In preparation for adding the GICv2m which requires address specifiers
and is a subnode of the gic, we extend the gic DT definition to specify
the #address-cells and #size-cells properties and add an empty ranges
property properties of the DT node, since this is required to add the
v2m node as a child of the gic node.

Note that we must also expand the irq-map to reference the gic with the
right address-cells as a consequence of this change.

Reviewed-by: Eric Auger <eric.auger@linaro.org>
Suggested-by: Shanker Donthineni <shankerd@codeaurora.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
Changes since v3:
 - Rewrote patch and changed authorship and tags accordingly
 - Fixed spelling in commit message
Changes since v2:
 - New separate patch factoring out changes to existing code for eased
   bisectability in case we broke something
 - The above fixes the issue with non-MSI compatible guests.

 hw/arm/virt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e5235ef..387dac8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -317,6 +317,9 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi)
                                      2, vbi->memmap[VIRT_GIC_DIST].size,
                                      2, vbi->memmap[VIRT_GIC_CPU].base,
                                      2, vbi->memmap[VIRT_GIC_CPU].size);
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
+    qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
@@ -585,7 +588,7 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
                                 int first_irq, const char *nodename)
 {
     int devfn, pin;
-    uint32_t full_irq_map[4 * 4 * 8] = { 0 };
+    uint32_t full_irq_map[4 * 4 * 10] = { 0 };
     uint32_t *irq_map = full_irq_map;
 
     for (devfn = 0; devfn <= 0x18; devfn += 0x8) {
@@ -598,13 +601,13 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
             uint32_t map[] = {
                 devfn << 8, 0, 0,                           /* devfn */
                 pin + 1,                                    /* PCI pin */
-                gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
+                gic_phandle, 0, 0, irq_type, irq_nr, irq_level }; /* GIC irq */
 
             /* Convert map to big endian */
-            for (i = 0; i < 8; i++) {
+            for (i = 0; i < 10; i++) {
                 irq_map[i] = cpu_to_be32(map[i]);
             }
-            irq_map += 8;
+            irq_map += 10;
         }
     }
 
-- 
2.1.2.330.g565301e.dirty

[Qemu-devel] [PATCH v4 4/4] target-arm: Add the GICv2m to the virt board

[Christoffer Dall]

Add a GICv2m device to the virt board to enable MSIs on the generic PCI
host controller.  We allocate 64 SPIs in the IRQ space for now (this can
be increased/decreased later) and map the GICv2m right after the GIC in
the memory map.

Reviewed-by: Eric Auger <eric.auger@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
Changes since v3:
 - Rebased on target-arm.next, so moved some definitions to virt.h
 - Added reviewed-by tag
Changes since v2:
 - Factored out changes to GIC DT node to previous patch.
 - Renamed QOM type name to "arm-gicv2m"
Changes since v1:
 - Remove stray merge conflict line
 - Reworded commmit message.

 hw/arm/virt.c         | 40 +++++++++++++++++++++++++++++++++++++++-
 include/hw/arm/virt.h |  2 ++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 387dac8..4bb7175 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -70,6 +70,7 @@ typedef struct VirtBoardInfo {
     int fdt_size;
     uint32_t clock_phandle;
     uint32_t gic_phandle;
+    uint32_t v2m_phandle;
 } VirtBoardInfo;
 
 typedef struct {
@@ -109,6 +110,7 @@ static const MemMapEntry a15memmap[] = {
     /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
     [VIRT_GIC_DIST] =   { 0x08000000, 0x00010000 },
     [VIRT_GIC_CPU] =    { 0x08010000, 0x00010000 },
+    [VIRT_GIC_V2M] =    { 0x08020000, 0x00001000 },
     [VIRT_UART] =       { 0x09000000, 0x00001000 },
     [VIRT_RTC] =        { 0x09010000, 0x00001000 },
     [VIRT_FW_CFG] =     { 0x09020000, 0x0000000a },
@@ -125,6 +127,7 @@ static const int a15irqmap[] = {
     [VIRT_RTC] = 2,
     [VIRT_PCIE] = 3, /* ... to 6 */
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+    [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
 };
 
 static VirtBoardInfo machines[] = {
@@ -300,9 +303,21 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
     }
 }
 
-static void fdt_add_gic_node(VirtBoardInfo *vbi)
+static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
 {
+    vbi->v2m_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+    qemu_fdt_add_subnode(vbi->fdt, "/intc/v2m");
+    qemu_fdt_setprop_string(vbi->fdt, "/intc/v2m", "compatible",
+                            "arm,gic-v2m-frame");
+    qemu_fdt_setprop(vbi->fdt, "/intc/v2m", "msi-controller", NULL, 0);
+    qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc/v2m", "reg",
+                                 2, vbi->memmap[VIRT_GIC_V2M].base,
+                                 2, vbi->memmap[VIRT_GIC_V2M].size);
+    qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
+}
 
+static void fdt_add_gic_node(VirtBoardInfo *vbi)
+{
     vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
     qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
 
@@ -323,6 +338,25 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi)
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
+static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    int i;
+    int irq = vbi->irqmap[VIRT_GIC_V2M];
+    DeviceState *dev;
+
+    dev = qdev_create(NULL, "arm-gicv2m");
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vbi->memmap[VIRT_GIC_V2M].base);
+    qdev_prop_set_uint32(dev, "base-spi", irq);
+    qdev_prop_set_uint32(dev, "num-spi", NUM_GICV2M_SPIS);
+    qdev_init_nofail(dev);
+
+    for (i = 0; i < NUM_GICV2M_SPIS; i++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+    }
+
+    fdt_add_v2m_gic_node(vbi);
+}
+
 static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
 {
     /* We create a standalone GIC v2 */
@@ -373,6 +407,8 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
     }
 
     fdt_add_gic_node(vbi);
+
+    create_v2m(vbi, pic);
 }
 
 static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -676,6 +712,8 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic)
     qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
                            nr_pcie_buses - 1);
 
+    qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", vbi->v2m_phandle);
+
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
                                  2, base_ecam, 2, size_ecam);
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ceec8b3..003ef29 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -32,6 +32,7 @@
 
 #include "qemu-common.h"
 
+#define NUM_GICV2M_SPIS       64
 #define NUM_VIRTIO_TRANSPORTS 32
 
 #define ARCH_TIMER_VIRT_IRQ   11
@@ -53,6 +54,7 @@ enum {
     VIRT_PCIE_MMIO,
     VIRT_PCIE_PIO,
     VIRT_PCIE_ECAM,
+    VIRT_GIC_V2M,
 };
 
 typedef struct MemMapEntry {
-- 
2.1.2.330.g565301e.dirty

Re: [Qemu-devel] [PATCH v4 0/4] Add support for for GICv2m and MSIs to arm-virt

[Peter Maydell]

On 29 May 2015 at 12:01, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> Now when we have a host generic PCIe controller in the virt board, it
> would be nice to be able to use MSIs so that we can eventually enable
> VHOST with KVM.
>
> With these patches you can use MSIs with TCG and with KVM, but you still
> need some fixes for the mapping of the IRQ index to the GSI number for
> IRQFD to work.  A separate series that enables IRQFD and vhost
> is available: "ARM adaptations for vhost irqfd setup"
> https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01054.html)
>
> Tested with KVM on XGene and with TCG by configuring a virtio-pci
> network adapter for the guest and verifying MSIs going through as
> expected.
>
> Rebased on target-arm.next, see the individual patches for detailed
> changelogs.

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Extend the gic node properties

[Claudio Fontana]

Hello Christoffer,

just one question:

> In preparation for adding the GICv2m which requires address specifiers
> and is a subnode of the gic, we extend the gic DT definition to specify
> the #address-cells and #size-cells properties and add an empty ranges
> property properties of the DT node, since this is required to add the
> v2m node as a child of the gic node.
>
> Note that we must also expand the irq-map to reference the gic with the
> right address-cells as a consequence of this change.
>
> Reviewed-by: Eric Auger <address@hidden>
> Suggested-by: Shanker Donthineni <address@hidden>
> Signed-off-by: Christoffer Dall <address@hidden>
> ---
> Changes since v3:
>  - Rewrote patch and changed authorship and tags accordingly
>  - Fixed spelling in commit message
> Changes since v2:
>  - New separate patch factoring out changes to existing code for eased
>    bisectability in case we broke something
>  - The above fixes the issue with non-MSI compatible guests.
>
>  hw/arm/virt.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e5235ef..387dac8 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -317,6 +317,9 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi)
>                                       2, vbi->memmap[VIRT_GIC_DIST].size,
>                                       2, vbi->memmap[VIRT_GIC_CPU].base,
>                                       2, vbi->memmap[VIRT_GIC_CPU].size);
> +    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
> +    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
> +    qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
>      qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
>  }
>  
> @@ -585,7 +588,7 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, 
> uint32_t gic_phandle,
>                                  int first_irq, const char *nodename)
>  {
>      int devfn, pin;
> -    uint32_t full_irq_map[4 * 4 * 8] = { 0 };
> +    uint32_t full_irq_map[4 * 4 * 10] = { 0 };
>      uint32_t *irq_map = full_irq_map;
>  
>      for (devfn = 0; devfn <= 0x18; devfn += 0x8) {
> @@ -598,13 +601,13 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, 
> uint32_t gic_phandle,
>              uint32_t map[] = {
>                  devfn << 8, 0, 0,                           /* devfn */
>                  pin + 1,                                    /* PCI pin */
> -                gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
> +                gic_phandle, 0, 0, irq_type, irq_nr, irq_level }; /* GIC irq */

can you help me understand how to decode this?
We are adding two 32bit "0" values in the map.
So now gic_phandle field occupies a total of 3 x 32bit values.
Is this really what was intended?

If the #address-cell and #size-cells of the intc are 2 and 2 respectively,
shouldn't this be

gic_phandle, 0, irq_type, irq_nr, irq_level ?

Too much time since I read this code I guess..

Thanks!

CLaudio

>  
>              /* Convert map to big endian */
> -            for (i = 0; i < 8; i++) {
> +            for (i = 0; i < 10; i++) {
>                  irq_map[i] = cpu_to_be32(map[i]);
>              }
> -            irq_map += 8;
> +            irq_map += 10;
>          }
>      }
>  
> -- 
> 2.1.2.330.g565301e.dirty
>

Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Extend the gic node properties

[Christoffer Dall]

[whoops, re-adding qemu-devel]

On Thu, Jul 9, 2015 at 5:07 PM, Alexander Graf <agraf@suse.de> wrote:
> On 07/09/15 16:44, Christoffer Dall wrote:
>>
>> On Thu, Jul 09, 2015 at 04:02:14PM +0200, Claudio Fontana wrote:
>>>
>>> Hello Christoffer,
>>>
>>> just one question:
>>>
>>>> In preparation for adding the GICv2m which requires address specifiers
>>>> and is a subnode of the gic, we extend the gic DT definition to specify
>>>> the #address-cells and #size-cells properties and add an empty ranges
>>>> property properties of the DT node, since this is required to add the
>>>> v2m node as a child of the gic node.
>>>>
>>>> Note that we must also expand the irq-map to reference the gic with the
>>>> right address-cells as a consequence of this change.
>>>>
>>>> Reviewed-by: Eric Auger <address@hidden>
>>>> Suggested-by: Shanker Donthineni <address@hidden>
>>>> Signed-off-by: Christoffer Dall <address@hidden>
>>>> ---
>>>> Changes since v3:
>>>>   - Rewrote patch and changed authorship and tags accordingly
>>>>   - Fixed spelling in commit message
>>>> Changes since v2:
>>>>   - New separate patch factoring out changes to existing code for eased
>>>>     bisectability in case we broke something
>>>>   - The above fixes the issue with non-MSI compatible guests.
>>>>
>>>>   hw/arm/virt.c | 11 +++++++----
>>>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>> index e5235ef..387dac8 100644
>>>> --- a/hw/arm/virt.c
>>>> +++ b/hw/arm/virt.c
>>>> @@ -317,6 +317,9 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi)
>>>>                                        2,
>>>> vbi->memmap[VIRT_GIC_DIST].size,
>>>>                                        2,
>>>> vbi->memmap[VIRT_GIC_CPU].base,
>>>>                                        2,
>>>> vbi->memmap[VIRT_GIC_CPU].size);
>>>> +    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
>>>> +    qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
>>>> +    qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
>>>>       qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle",
>>>> vbi->gic_phandle);
>>>>   }
>>>>   @@ -585,7 +588,7 @@ static void create_pcie_irq_map(const
>>>> VirtBoardInfo *vbi,
>>>> uint32_t gic_phandle,
>>>>                                   int first_irq, const char *nodename)
>>>>   {
>>>>       int devfn, pin;
>>>> -    uint32_t full_irq_map[4 * 4 * 8] = { 0 };
>>>> +    uint32_t full_irq_map[4 * 4 * 10] = { 0 };
>>>>       uint32_t *irq_map = full_irq_map;
>>>>         for (devfn = 0; devfn <= 0x18; devfn += 0x8) {
>>>> @@ -598,13 +601,13 @@ static void create_pcie_irq_map(const
>>>> VirtBoardInfo *vbi,
>>>> uint32_t gic_phandle,
>>>>               uint32_t map[] = {
>>>>                   devfn << 8, 0, 0,                           /* devfn
>>>> */
>>>>                   pin + 1,                                    /* PCI pin
>>>> */
>>>> -                gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq
>>>> */
>>>> +                gic_phandle, 0, 0, irq_type, irq_nr, irq_level }; /*
>>>> GIC irq */
>>>
>>> can you help me understand how to decode this?
>>> We are adding two 32bit "0" values in the map.
>>> So now gic_phandle field occupies a total of 3 x 32bit values.
>>> Is this really what was intended?
>>>
>>> If the #address-cell and #size-cells of the intc are 2 and 2
>>> respectively,
>>> shouldn't this be
>>>
>>> gic_phandle, 0, irq_type, irq_nr, irq_level ?
>>>
>>> Too much time since I read this code I guess..
>>>
>> I'll be honest and say that I don't fully understand the details of the
>> interrupt-map specification, and I cannot seem to find it online either
>> (the working link I had before gives me a 404 these days).
>>
>> So I followed the suggestion from Shanker and it worked with adding two
>> zeroes, not with only adding one, so I figured it was the correct thing
>> to do.
>>
>> Perhaps Alex Graf who wrote the original code to generate the interrupt
>> map can help?
>
>
> The page works quite well for me:
>
>   http://devicetree.org/Device_Tree_Usage#Advanced_Interrupt_Mapping

It explains it conceptually, yes, but it's hardly a spec.  At least I
can't understand from that page why the entries in the map have to be
changed based on size-cells and address-cells in the interrupt
controller...

I didn't think the bits we needed to add were related to the phandle;
I always thought a phandle was just an internal to the DT 32-bit
number to refer to a different node.

>
> When it comes to dt bits where I'm uncertain, I usually end up asking Rob
> though :).
>

Rob, can you help us out here?  Beers are on me at SFO15 ;)

-Christoffer

Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Extend the gic node properties

[Peter Maydell]

On 9 July 2015 at 17:03, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> [whoops, re-adding qemu-devel]
>
> On Thu, Jul 9, 2015 at 5:07 PM, Alexander Graf <agraf@suse.de> wrote:
>> On 07/09/15 16:44, Christoffer Dall wrote:
>>> I'll be honest and say that I don't fully understand the details of the
>>> interrupt-map specification, and I cannot seem to find it online either
>>> (the working link I had before gives me a 404 these days).

Try http://www.firmware.org/1275/practice/imap/imap0_9d.pdf

> It explains it conceptually, yes, but it's hardly a spec.  At least I
> can't understand from that page why the entries in the map have to be
> changed based on size-cells and address-cells in the interrupt
> controller...

The interrupt map entries are:
 * child unit interrupt specifier [4 cells for PCI, determined
   by #address-cells + #interrupt-cells for the PCI controller node]
 * interrupt parent phandle
 * parent unit interrupt specifier [number of cells determined
   by #address-cells + #interrupt-cells for the interrupt controller]

So the extra two zeroes aren't part of the phandle, they're
the result of the parent unit-interrupt-specifier now being
5 cells because of #address-cells being defined in the GIC node.

(https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/gic.txt
is unfortunately not clear about what the extra two cells
in the unit-interrupt-specifier are actually for.)

> I didn't think the bits we needed to add were related to the phandle;
> I always thought a phandle was just an internal to the DT 32-bit
> number to refer to a different node.

Yep.

-- PMM

Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Extend the gic node properties

[Christoffer Dall]

On Thu, Jul 9, 2015 at 6:28 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 July 2015 at 17:03, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> [whoops, re-adding qemu-devel]
>>
>> On Thu, Jul 9, 2015 at 5:07 PM, Alexander Graf <agraf@suse.de> wrote:
>>> On 07/09/15 16:44, Christoffer Dall wrote:
>>>> I'll be honest and say that I don't fully understand the details of the
>>>> interrupt-map specification, and I cannot seem to find it online either
>>>> (the working link I had before gives me a 404 these days).
>
> Try http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
>

that's the one, thanks for the working url.

>> It explains it conceptually, yes, but it's hardly a spec.  At least I
>> can't understand from that page why the entries in the map have to be
>> changed based on size-cells and address-cells in the interrupt
>> controller...
>
> The interrupt map entries are:
>  * child unit interrupt specifier [4 cells for PCI, determined
>    by #address-cells + #interrupt-cells for the PCI controller node]
>  * interrupt parent phandle
>  * parent unit interrupt specifier [number of cells determined
>    by #address-cells + #interrupt-cells for the interrupt controller]
>
> So the extra two zeroes aren't part of the phandle, they're
> the result of the parent unit-interrupt-specifier now being
> 5 cells because of #address-cells being defined in the GIC node.

Thanks for clearing this up!

I owe you a cup of your favorite poison then:)

-Christoffer