Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Bharat Kumar Gogada <bharatku@xilinx.com>
To:     Marc Zyngier <maz@kernel.org>,
        "lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>
CC:     Frank Wunderlich <frank-w@public-files.de>,
        Thierry Reding <treding@nvidia.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Rob Herring <robh@kernel.org>, Will Deacon <will@kernel.org>,
        "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Michael Kelley <mikelley@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Ryder Lee <ryder.lee@mediatek.com>,
        Marek Vasut <marek.vasut+renesas@gmail.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        Michal Simek <michals@xilinx.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "linux-mediatek@lists.infradead.org" 
        <linux-mediatek@lists.infradead.org>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        "kernel-team@android.com" <kernel-team@android.com>
Subject: RE: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
Thread-Topic: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
Thread-Index: AQHXH0vIUXC892kDXkW3zUak9HM8+aqTE3EQ
Date:   Wed, 24 Mar 2021 12:42:24 +0000
Message-ID: <BYAPR02MB5559A0B0DA88866EDC7BDFE5A5639@BYAPR02MB5559.namprd02.prod.outlook.com>
References: <20210322184614.802565-1-maz@kernel.org>
 <20210322184614.802565-6-maz@kernel.org>
In-Reply-To: <20210322184614.802565-6-maz@kernel.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 2594ff9b-4419-44ef-03d1-08d8eec246c5
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2021 12:42:24.8759
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: CVrrKU6jxdP540Y8lOK3my9rbhFLFELgCtmpNzKU3jOcisQo3pizXC/Yt09ZuymLYdWSf9w8z5KugnjguTMn6Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR02MB6419
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Marc,

Thanks for the patch.=20

> Subject: [PATCH v2 05/15] PCI: xilinx: Convert to MSI domains
>=20
> In anticipation of the removal of the msi_controller structure, convert t=
he
> ancient xilinx host controller driver to MSI domains.
>=20
> We end-up with the usual two domain structure, the top one being a generi=
c
> PCI/MSI domain, the bottom one being xilinx-specific and handling the
> actual HW interrupt allocation.
>=20
> This allows us to fix some of the most appalling MSI programming, where t=
he
> 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. Th=
is
> should allow support for MultiMSI, but I decided not to even try, since I
> cannot test it.
>=20
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/Kconfig       |   2 +-
>  drivers/pci/controller/pcie-xilinx.c | 234 +++++++++++----------------
>  2 files changed, 97 insertions(+), 139 deletions(-)
>=20
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kcon=
fig
> index 5cc07d28a3a0..60045f7aafc5 100644
...


> +static struct irq_chip xilinx_msi_bottom_chip =3D {
> +	.name			=3D "Xilinx MSI",
> +	.irq_set_affinity 	=3D xilinx_msi_set_affinity,
> +	.irq_compose_msi_msg	=3D xilinx_compose_msi_msg,
> +};
>=20
I see a crash while testing MSI in handle_edge_irq
[<c015bdd4>] (handle_edge_irq) from [<c0157164>] (generic_handle_irq+0x28/0=
x38)
[<c0157164>] (generic_handle_irq) from [<c03a9714>] (xilinx_pcie_intr_handl=
er+0x17c/0x2b0)
[<c03a9714>] (xilinx_pcie_intr_handler) from [<c0157d94>] (__handle_irq_eve=
nt_percpu+0x3c/0xc0)
[<c0157d94>] (__handle_irq_event_percpu) from [<c0157e44>] (handle_irq_even=
t_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 i=
nt
> virq,
> +				  unsigned int nr_irqs, void *args)
>  {
> -	struct xilinx_pcie_port *port =3D pdev->bus->sysdata;
> -	unsigned int irq;
> -	int hwirq;
> -	struct msi_msg msg;
> -	phys_addr_t msg_addr;
> +	struct xilinx_pcie_port *port =3D domain->host_data;
> +	int hwirq, i;
> +
> +	mutex_lock(&port->map_lock);
> +
> +	hwirq =3D bitmap_find_free_region(port->msi_map,
> XILINX_NUM_MSI_IRQS,
> +order_base_2(nr_irqs));
> +
> +	mutex_unlock(&port->map_lock);
>=20
> -	hwirq =3D xilinx_pcie_assign_msi();
>  	if (hwirq < 0)
> -		return hwirq;
> +		return -ENOSPC;
>=20
> -	irq =3D irq_create_mapping(port->msi_domain, hwirq);
> -	if (!irq)
> -		return -EINVAL;
> +	for (i =3D 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);
>=20
> -	irq_set_msi_desc(irq, desc);
> +	return 0;
> +}
>

Regards,
Bharat=20