From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBOHr-0006YA-KE for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:47:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YBOHo-0004U3-Bg for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:47:07 -0500 Received: from mail-ie0-f171.google.com ([209.85.223.171]:64129) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBOHo-0004Tz-6B for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:47:04 -0500 Received: by mail-ie0-f171.google.com with SMTP id ar1so8799388iec.2 for ; Wed, 14 Jan 2015 05:47:03 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <54B66AAF.7020904@huawei.com> References: <1421230611-12481-1-git-send-email-a.rigo@virtualopensystems.com> <1421230611-12481-3-git-send-email-a.rigo@virtualopensystems.com> <54B66AAF.7020904@huawei.com> Date: Wed, 14 Jan 2015 14:47:03 +0100 Message-ID: From: alvise rigo Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Claudio Fontana Cc: Peter Maydell , VirtualOpenSystems Technical Team , QEMU Developers , Rob Herring , Alexander Graf Hi Claudio, On Wed, Jan 14, 2015 at 2:10 PM, Claudio Fontana wrote: > On 14.01.2015 11:16, Alvise Rigo wrote: >> The platform memory map has now three more memory ranges to map the >> device's memory regions (Configuration region, I/O region and Memory >> region). >> >> The dt node interrupt-map property tells how to route the PCI interrupts >> to system interrupts. In the mach-virt case, four IRQs are swizzled >> between all the possible interrupts coming from the PCI bus. In >> particular, the mapping ensures that the first four devices will use a >> system IRQ always different (supposing that only PIN_A of each device is >> used). >> >> Now that a PCI bus is provided, the machine can be launched with >> multiple PCI devices through the -device option (e.g., -device >> virtio-blk-pci). >> >> Signed-off-by: Alvise Rigo >> --- >> hw/arm/virt.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++- >> 1 file changed, 111 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 2353440..7b0326f 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -44,6 +44,7 @@ >> #include "qemu/error-report.h" >> >> #define NUM_VIRTIO_TRANSPORTS 32 >> +#define NUM_PCI_IRQS 4 >> >> /* Number of external interrupt lines to configure the GIC with */ >> #define NUM_IRQS 128 >> @@ -68,8 +69,12 @@ enum { >> VIRT_UART, >> VIRT_MMIO, >> VIRT_RTC, >> + VIRT_PCI_CFG, >> + VIRT_PCI_IO, >> + VIRT_PCI_MEM, >> VIRT_FW_CFG, >> }; >> +#define VIRT_PCI VIRT_PCI_CFG >> >> typedef struct MemMapEntry { >> hwaddr base; >> @@ -129,13 +134,17 @@ static const MemMapEntry a15memmap[] =3D { >> [VIRT_FW_CFG] =3D { 0x09020000, 0x0000000a }, >> [VIRT_MMIO] =3D { 0x0a000000, 0x00000200 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that = size */ >> - /* 0x10000000 .. 0x40000000 reserved for PCI */ >> + /* PCI regions */ >> + [VIRT_PCI_CFG] =3D { 0x10000000, 0x01000000 }, >> + [VIRT_PCI_IO] =3D { 0x11000000, 0x00010000 }, >> + [VIRT_PCI_MEM] =3D { 0x12000000, 0x2e000000 }, >> [VIRT_MEM] =3D { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, >> }; >> >> static const int a15irqmap[] =3D { >> [VIRT_UART] =3D 1, >> [VIRT_RTC] =3D 2, >> + [VIRT_PCI] =3D 3, /* ...to 3 + NUM_PCI_IRQS - 1 */ >> [VIRT_MMIO] =3D 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >> }; >> >> @@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, q= emu_irq *pic) >> g_free(nodename); >> } >> >> +static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic) >> +{ >> + DeviceState *dev; >> + SysBusDevice *busdev; >> + int i, j, intmap_rows; >> + const MemMapEntry *mme =3D vbi->memmap; >> + uint32_t plat_acells; >> + uint32_t plat_scells; >> + uint32_t gic_phandle; >> + char *nodename; >> + >> + dev =3D qdev_create(NULL, "generic_pci"); >> + qdev_prop_set_uint64(dev, "mmio_win_size", mme[VIRT_PCI_MEM].size); >> + qdev_prop_set_uint64(dev, "mmio_win_addr", mme[VIRT_PCI_MEM].base); >> + qdev_prop_set_uint32(dev, "irqs", NUM_PCI_IRQS); >> + qdev_init_nofail(dev); >> + >> + busdev =3D SYS_BUS_DEVICE(dev); >> + sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config *= / >> + sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base); /* PCI I/O */ >> + sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory w= indow */ >> + >> + for ( i =3D 0; i < NUM_PCI_IRQS; i++) { >> + sysbus_connect_irq(busdev, i, pic[vbi->irqmap[VIRT_PCI] + i]); >> + } >> + >> + /* add device tree node */ >> + nodename =3D g_strdup_printf("/pci@%" PRIx64, mme[VIRT_PCI_CFG].bas= e); >> + qemu_fdt_add_subnode(vbi->fdt, nodename); >> + qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", >> + "pci-host-cam-generic"); >> + qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci"); >> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 0x3); >> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 0x2); >> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 0x1); >> + >> + plat_acells =3D qemu_fdt_getprop_cell(vbi->fdt, "/", "#address-cell= s"); >> + plat_scells =3D qemu_fdt_getprop_cell(vbi->fdt, "/", "#size-cells")= ; >> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", >> + plat_acells, mme[VIRT_PCI_CFG].base= , >> + plat_scells, mme[VIRT_PCI_CFG].size= ); >> + >> + /* Two regions (second and third of *busdev): >> + - I/O region, PCI addr =3D 0x0, CPU addr =3D mme[VIRT_PCI_IO].ba= se, >> + size =3D mme[VIRT_PCI_IO].size, >> + - 32bit MMIO region, PCI addr =3D CPU addr =3D mme[VIRT_PCI_MEM]= .base, >> + size =3D mme[VIRT_PCI_MEM].size */ >> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges", >> + 1, 0x01000000, 2, 0x00000000, /* I/O space */ >> + 2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size, >> + 1, 0x02000000, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory s= pace */ >> + 2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size); >> + >> + gic_phandle =3D qemu_fdt_get_phandle(vbi->fdt, "/intc"); >> + >> +#define IRQ_MAP_ENTRY_DESC_SZ 14 >> +#define PHYSH_DEV_SHIFT 11 >> + /* Any interrupt from the PCI bus is represented by four 32bit valu= es > + physM physL pin#> (unit-interrupt-specifier), where bits physH[1= 1:15] >> + identify the device number and pin# the pin number that triggere= d. We use >> + the two bits physH[14:15] and pin# to route all the PCI interrup= ts to >> + four different system interrupts. The following mask is used to = honour >> + exactly those bits. */ >> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mas= k", >> + 1, 0x3 << PHYSH_DEV_SHIFT, 1, 0x0, 1, 0x0, 1, 0x7); >> + >> + /* interrupt-map property >> + Two bits of physH[14:15] and four possible values of pin# give a= maximum >> + of 16 different results of the mask operation: >> + AND >> + Cycling through the four system interrupts available, we assign = to each >> + result an interrupt number (basically, 32 * 4 maximum PCI interr= upts are >> + mapped to 4 system interrupts). >> + Each entry inside interrupt-map has the following form: >> + = */ >> + intmap_rows =3D 16; >> + uint64_t *int_mapping_data =3D g_malloc0(IRQ_MAP_ENTRY_DESC_SZ * >> + sizeof(uint64_t) * intmap_rows); >> + uint64_t *int_map_data =3D int_mapping_data; >> + for (i =3D 0; i < 4; i++) { >> + for (j =3D 0; j < 4; j++) { >> + uint64_t physh_dev_nr =3D i << PHYSH_DEV_SHIFT; >> + uint64_t pci_pin_nr =3D j + 1; >> + uint64_t row[IRQ_MAP_ENTRY_DESC_SZ] =3D >> + {1, physh_dev_nr, 2, 0x0, 1, pci_pin_nr, // masked PCI inter= rupt... >> + 1, gic_phandle, 1, GIC_FDT_IRQ_TYPE_SPI, // ...mapped to th= is IRQ >> + 1, (vbi->irqmap[VIRT_PCI_CFG] + (j + i) % 4), >> + 1, GIC_FDT_IRQ_FLAGS_LEVEL_HI}; >> + int_map_data =3D mempcpy(int_map_data, row, sizeof(row)); >> + } >> + } >> + > > sorry for maybe being pedantic, but is this working as intended? Yes, four interrupts are used for all the PCI IRQs. > The same PHYS_HI is mapped multiple times to a different irq if I underst= and this correctly, ending up in these mappings: > > B,D,F irqmap-mask 0x00001800 > PHYS_HI > SPI irq mappings > > 0x00000000 -> SPI irq 3 > 0x00000000 -> SPI irq 4 > 0x00000000 -> SPI irq 5 > 0x00000000 -> SPI irq 6 > > 0x00000800 -> SPI irq 3 > 0x00000800 -> SPI irq 4 > 0x00000800 -> SPI irq 5 > 0x00000800 -> SPI irq 6 > > 0x00001000 -> SPI irq 3 > 0x00001000 -> SPI irq 4 > 0x00001000 -> SPI irq 5 > 0x00001000 -> SPI irq 6 > > 0x00001800 -> SPI irq 3 > 0x00001800 -> SPI irq 4 > 0x00001800 -> SPI irq 5 > 0x00001800 -> SPI irq 6 The interrupt-map generated is the following: 0x0 0x0 0x0 0x1 0x8001 0x0 0x3 0x4 0x0 0x0 0x0 0x2 0x8001 0x0 0x4 0x4 0x0 0x0 0x0 0x3 0x8001 0x0 0x5 0x4 0x0 0x0 0x0 0x4 0x8001 0x0 0x6 0x4 0x800 0x0 0x0 0x1 0x8001 0x0 0x4 0x4 0x800 0x0 0x0 0x2 0x8001 0x0 0x5 0x4 0x800 0x0 0x0 0x3 0x8001 0x0 0x6 0x4 0x800 0x0 0x0 0x4 0x8001 0x0 0x3 0x4 0x1000 0x0 0x0 0x1 0x8001 0x0 0x5 0x4 0x1000 0x0 0x0 0x2 0x8001 0x0 0x6 0x4 0x1000 0x0 0x0 0x3 0x8001 0x0 0x3 0x4 0x1000 0x0 0x0 0x4 0x8001 0x0 0x4 0x4 0x1800 0x0 0x0 0x1 0x8001 0x0 0x6 0x4 0x1800 0x0 0x0 0x2 0x8001 0x0 0x3 0x4 0x1800 0x0 0x0 0x3 0x8001 0x0 0x4 0x4 0x1800 0x0 0x0 0x4 0x8001 0x0 0x5 0x4 Now, removing the INT_{B,C,D} information we have: 0x0 0x0 0x0 0x1 0x8001 0x0 0x3 0x4 0x800 0x0 0x0 0x1 0x8001 0x0 0x4 0x4 0x1000 0x0 0x0 0x1 0x8001 0x0 0x5 0x4 0x1800 0x0 0x0 0x1 0x8001 0x0 0x6 0x4 Which means that, when the two lower bits of the device number are "00" (and so the mask result is 0x0 0x0 0x0 0x1, as the first entry of the interrupt-map), the assigned IRQ will be 3. If the two lower bit are "01" (mask result 0x800 0x0 0x0 0x1), the assigned IRQ number will be 4, and so on. Regards, alvise > > did you not intend to do > > B,D,F irqmap-mask 0x00001800 > PHYS_HI > SPI irq mappings > > 0x00000000 -> SPI irq 3 > 0x00000800 -> SPI irq 4 > 0x00001000 -> SPI irq 5 > 0x00001800 -> SPI irq 6 > > ? > > Thanks, > > Claudio > > >> + qemu_fdt_setprop_sized_cells_from_array(vbi->fdt, nodename, "interr= upt-map", >> + (intmap_rows * IRQ_MAP_ENTRY_DESC_SZ)/2, int_mappi= ng_data); >> + >> + g_free(int_mapping_data); >> + g_free(nodename); >> +} >> + >> static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *p= ic) >> { >> int i; >> @@ -640,6 +748,8 @@ static void machvirt_init(MachineState *machine) >> >> create_rtc(vbi, pic); >> >> + create_pci_host(vbi, pic); >> + >> /* Create mmio transports, so the user can create virtio backends >> * (which will be automatically plugged in to the transports). If >> * no backend is created the transport will just sit harmlessly idl= e. >> > > > -- > Claudio Fontana > Server Virtualization Architect > Huawei Technologies Duesseldorf GmbH > Riesstra=C3=9Fe 25 - 80992 M=C3=BCnchen > > office: +49 89 158834 4135 > mobile: +49 15253060158