Hi Marc,
Thanks for the patch.
> Subject: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
>
> In anticipation of the removal of the msi_controller structure, convert the
> ancient xilinx host controller driver to MSI domains.
>
> We end-up with the usual two domain structure, the top one being a generic
> PCI/MSI domain, the bottom one being xilinx-specific and handling the
> actual HW interrupt allocation.
>
> This allows us to fix some of the most appalling MSI programming, where the
> message programmed in the device is the virtual IRQ number instead of the
> allocated vector number. The allocator is also made safe with a mutex. This
> should allow support for MultiMSI, but I decided not to even try, since I
> cannot test it.
>
> Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> drivers/pci/controller/Kconfig | 2 +-
> drivers/pci/controller/pcie-xilinx.c | 234 +++++++++++----------------
> 2 files changed, 97 insertions(+), 139 deletions(-)
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 5cc07d28a3a0..60045f7aafc5 100644
...
> +static struct irq_chip xilinx_msi_bottom_chip = {
> + .name = "Xilinx MSI",
> + .irq_set_affinity = xilinx_msi_set_affinity,
> + .irq_compose_msi_msg = xilinx_compose_msi_msg,
> +};
>
I see a crash while testing MSI in handle_edge_irq
[<c015bdd4>] (handle_edge_irq) from [<c0157164>] (generic_handle_irq+0x28/0x38)
[<c0157164>] (generic_handle_irq) from [<c03a9714>] (xilinx_pcie_intr_handler+0x17c/0x2b0)
[<c03a9714>] (xilinx_pcie_intr_handler) from [<c0157d94>] (__handle_irq_event_percpu+0x3c/0xc0)
[<c0157d94>] (__handle_irq_event_percpu) from [<c0157e44>] (handle_irq_event_percpu+0x2c/0x7c)
[<c0157e44>] (handle_irq_event_percpu) from [<c0157ecc>] (handle_irq_event+0x38/0x5c)
[<c0157ecc>] (handle_irq_event) from [<c015bc8c>] (handle_fasteoi_irq+0x9c/0x114)
void handle_edge_irq(struct irq_desc *desc) {
...
kstat_incr_irqs_this_cpu(desc);
/* Start handling the irq */
desc->irq_data.chip->irq_ack(&desc->irq_data); //There is no check here for ack function is registered for chip
..
}
> -/**
> - * xilinx_pcie_msi_setup_irq - Setup MSI request
> - * @chip: MSI chip pointer
> - * @pdev: PCIe device pointer
> - * @desc: MSI descriptor pointer
> - *
> - * Return: '0' on success and error value on failure
> - */
> -static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
> - struct pci_dev *pdev,
> - struct msi_desc *desc)
> +static int xilinx_msi_domain_alloc(struct irq_domain *domain, unsigned int
> virq,
> + unsigned int nr_irqs, void *args)
> {
> - struct xilinx_pcie_port *port = pdev->bus->sysdata;
> - unsigned int irq;
> - int hwirq;
> - struct msi_msg msg;
> - phys_addr_t msg_addr;
> + struct xilinx_pcie_port *port = domain->host_data;
> + int hwirq, i;
> +
> + mutex_lock(&port->map_lock);
> +
> + hwirq = bitmap_find_free_region(port->msi_map,
> XILINX_NUM_MSI_IRQS,
> +order_base_2(nr_irqs));
> +
> + mutex_unlock(&port->map_lock);
>
> - hwirq = xilinx_pcie_assign_msi();
> if (hwirq < 0)
> - return hwirq;
> + return -ENOSPC;
>
> - irq = irq_create_mapping(port->msi_domain, hwirq);
> - if (!irq)
> - return -EINVAL;
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_info(domain, virq + i, hwirq + i,
> + &xilinx_msi_bottom_chip, domain-
> >host_data,
> + handle_edge_irq, NULL, NULL);
>
> - irq_set_msi_desc(irq, desc);
> + return 0;
> +}
>
Regards,
Bharat
