Re: [Qemu-devel] [PATCH 2/2] riscv: virt: Fix interrupt mapping

[Alistair Francis]

On Tue, Nov 20, 2018 at 3:01 PM Guenter Roeck <address@hidden> wrote:
>
> - Interrupt map width is 6, not 7
>         address (3), pci interrupt (1), controller phandle (1), irq (1)
> - Since the interrupt map is the default pci interrupt map, we can not
>   omit devfn from the mapping function.
> - It is not necessary to specify "interrupt-parent" and "interrupts"
>   since both are part of interrupt-map.
>
> Signed-off-by: Guenter Roeck <address@hidden>

Hey Gunter,

Thanks again for the patch!

Also unfortunately Logan Gunthorpe had already sent me fixes for the
interrupt mappings as well. You can see his tree here:
https://github.com/lsgunth/qemu/commits/riscv_pci

If you have comments on that implementation just let me know.
Hopefully I will be able to send out a new version tomorrow.

I really appreciate your help! It's always good to have multiple
people working and looking over everything.

Alistair

> ---
>  hw/riscv/virt.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 675899d..fb3a492 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -65,35 +65,37 @@ static const struct MemmapEntry {
>      [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 },
>  };
>
> -#define INTERREUPT_MAP_WIDTH 7
> +#define INTERRUPT_MAP_WIDTH 6
>
>  static void create_pcie_irq_map(void *fdt, char *nodename,
>                                  uint32_t plic_phandle)
>  {
> -    int pin;
> -    uint32_t full_irq_map[GPEX_NUM_IRQS * INTERREUPT_MAP_WIDTH] = { 0 };
> +    int devfn, pin;
> +    uint32_t full_irq_map[GPEX_NUM_IRQS * 4 * INTERRUPT_MAP_WIDTH] = { 0 };
>      uint32_t *irq_map = full_irq_map;
>
> +    for (devfn = 0; devfn <= 0x18; devfn += 0x8) {
>          for (pin = 0; pin < GPEX_NUM_IRQS; pin++) {
> -            int irq_nr = PCIE_IRQ + (pin % PCI_NUM_PINS);
> +            int irq_nr = PCIE_IRQ + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
>              int i;
>
> -            uint32_t map[] = {
> -                0, 0, 0,
> -                pin + 1, plic_phandle, 0, irq_nr};
> +            uint32_t map[INTERRUPT_MAP_WIDTH] = {
> +                devfn << 8, 0, 0,
> +                pin + 1, plic_phandle, irq_nr};
>
>              /* Convert map to big endian */
> -            for (i = 0; i < INTERREUPT_MAP_WIDTH; i++) {
> +            for (i = 0; i < INTERRUPT_MAP_WIDTH; i++) {
>                  irq_map[i] = cpu_to_be32(map[i]);
>              }
> -            irq_map += INTERREUPT_MAP_WIDTH;
> +            irq_map += INTERRUPT_MAP_WIDTH;
>          }
> +    }
>
>      qemu_fdt_setprop(fdt, nodename, "interrupt-map",
>                       full_irq_map, sizeof(full_irq_map));
>
>      qemu_fdt_setprop_cells(fdt, nodename, "interrupt-map-mask",
> -                           0, 0, 0, 0x7);
> +                           0x1800, 0, 0, 0x7);
>  }
>
>  static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
> @@ -261,8 +263,6 @@ static void *create_fdt(RISCVVirtState *s, const struct 
> MemmapEntry *memmap,
>                                   2, memmap[VIRT_PCIE_MMIO_HIGH].base,
>                                   2, memmap[VIRT_PCIE_MMIO_HIGH].size);
>
> -    qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", plic_phandle);
> -    qemu_fdt_setprop_cells(fdt, nodename, "interrupts", PCIE_IRQ);
>      create_pcie_irq_map(fdt, nodename, plic_phandle);
>
>      nodename = g_strdup_printf("/address@hidden",
> --
> 2.7.4
>
>