linux-kernel - RE: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BYAPR02MB5559A0B0DA88866EDC7BDFE5A5639@BYAPR02MB5559.namprd02.prod.outlook.com>
Date:   Wed, 24 Mar 2021 12:42:24 +0000
From:   Bharat Kumar Gogada <bharatku@...inx.com>
To:     Marc Zyngier <maz@...nel.org>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
CC:     Frank Wunderlich <frank-w@...lic-files.de>,
        Thierry Reding <treding@...dia.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Rob Herring <robh@...nel.org>, Will Deacon <will@...nel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Michael Kelley <mikelley@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Ryder Lee <ryder.lee@...iatek.com>,
        Marek Vasut <marek.vasut+renesas@...il.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Michal Simek <michals@...inx.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        "kernel-team@...roid.com" <kernel-team@...roid.com>
Subject: RE: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains

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@...gle.com>
> Signed-off-by: Marc Zyngier <maz@...nel.org>
> ---
>  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