| Message ID | 1428528060-17896-4-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 8 April 2015 at 22:21, Christoffer Dall <christoffer.dall@linaro.org> wrote: > Adding the GICv2m to the virt board should allow us to enable MSIs on > the generic PCI host controller, in theory. > So is this commit message just saying "I haven't tested this patchset" :-), or are we still missing some functionality to get things working? (kernel or qemu?) > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > hw/arm/virt.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index e01676a..0c8dae9 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -45,6 +45,7 @@ > #include "hw/pci-host/gpex.h" > > #define NUM_VIRTIO_TRANSPORTS 32 > +#define NUM_GICV2M_SPIS 64 > > /* Number of external interrupt lines to configure the GIC with */ > #define NUM_IRQS 128 > @@ -71,6 +72,7 @@ enum { > VIRT_RTC, > VIRT_FW_CFG, > VIRT_PCIE, > + VIRT_GIC_V2M, > }; > > typedef struct MemMapEntry { > @@ -88,6 +90,7 @@ typedef struct VirtBoardInfo { > int fdt_size; > uint32_t clock_phandle; > uint32_t gic_phandle; > + uint32_t v2m_phandle; > } VirtBoardInfo; > > typedef struct { > @@ -127,6 +130,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 }, > @@ -148,6 +152,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[] = { > @@ -323,9 +328,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); > > @@ -340,9 +357,31 @@ 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); > Why do we need an empty ranges attribute? 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, "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 */ > @@ -391,6 +430,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) > @@ -699,6 +740,10 @@ static void create_pcie(const VirtBoardInfo *vbi, > qemu_irq *pic) > qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); > qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, > nr_pcie_buses - 1); > + qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, > + nr_pcie_buses - 1); > Merge conflict misresolution? + > + 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); > -- > 2.1.2.330.g565301e.dirty > > > thanks -- PMM
On Tue, Apr 21, 2015 at 03:47:13PM +0100, Peter Maydell wrote: > On 8 April 2015 at 22:21, Christoffer Dall <christoffer.dall@linaro.org> > wrote: > > > Adding the GICv2m to the virt board should allow us to enable MSIs on > > the generic PCI host controller, in theory. > > > > So is this commit message just saying "I haven't tested this > patchset" :-), or are we still missing some functionality to get > things working? (kernel or qemu?) I basically forgot to revise the commit text after the initial prototype work. We still need Eric's patches for this to work with IRQFD and VHOST, but MSIs actually do work with this code on their own. And I did test the patch set. I will revise the commit text. -Christoffer
On Tue, Apr 21, 2015 at 03:47:13PM +0100, Peter Maydell wrote: [...] > > @@ -340,9 +357,31 @@ 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); > > > > Why do we need an empty ranges attribute? > Without it, Linux fails to make things work. I suspect this is related to specifically setting the #address-cells and #size-cells. I forgot by now, but I think when I originally wrote this patch I traced through the Linux code and found out that it was required somewhere. I also think the AMD GICv2m FDT has the empty ranges property. -Christoffer
On 27 April 2015 at 17:06, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Tue, Apr 21, 2015 at 03:47:13PM +0100, Peter Maydell wrote: > > [...] > >> > @@ -340,9 +357,31 @@ 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); >> > >> >> Why do we need an empty ranges attribute? >> > > Without it, Linux fails to make things work. I suspect this is related > to specifically setting the #address-cells and #size-cells. Looking at the DT spec I think it's a requirement for any node which has children, to specify the address translation. Empty ranges means "1:1 translation" (and more complicated transformations are possible). -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index e01676a..0c8dae9 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -45,6 +45,7 @@ #include "hw/pci-host/gpex.h" #define NUM_VIRTIO_TRANSPORTS 32 +#define NUM_GICV2M_SPIS 64 /* Number of external interrupt lines to configure the GIC with */ #define NUM_IRQS 128 @@ -71,6 +72,7 @@ enum { VIRT_RTC, VIRT_FW_CFG, VIRT_PCIE, + VIRT_GIC_V2M, }; typedef struct MemMapEntry { @@ -88,6 +90,7 @@ typedef struct VirtBoardInfo { int fdt_size; uint32_t clock_phandle; uint32_t gic_phandle; + uint32_t v2m_phandle; } VirtBoardInfo; typedef struct { @@ -127,6 +130,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 }, @@ -148,6 +152,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[] = { @@ -323,9 +328,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); @@ -340,9 +357,31 @@ 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); } +static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic) +{ + int i; + int irq = vbi->irqmap[VIRT_GIC_V2M]; + DeviceState *dev; + + dev = qdev_create(NULL, "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 */ @@ -391,6 +430,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) @@ -699,6 +740,10 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic) qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, nr_pcie_buses - 1); + 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);
Adding the GICv2m to the virt board should allow us to enable MSIs on the generic PCI host controller, in theory. Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> --- hw/arm/virt.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)