+ Marc
On Wed, Jun 19, 2024 at 04:28:29PM +0200, Marek Behún wrote:
> From: Pali Rohár <pali@xxxxxxxxxx>
>
> Documentation for irq_domain_remove() says that all mapping within the
> domain must be disposed prior to domain remove.
>
> Currently INTx irqs are not disposed in pci-mvebu.c device unbind callback
> which cause that kernel crashes after unloading driver and trying to read
> /sys/kernel/debug/irq/irqs/<num> or /proc/interrupts.
>
Thanks a lot for respinning this patch. It is indeed fixing a real problem since
this driver can be unloaded runtime.
It is always a debate on whether an irqchip controller should be allowed to be
removed runtime or not, but I hope Marc will provide some inputs here.
> Fixes: ec075262648f ("PCI: mvebu: Implement support for legacy INTx interrupts")
> Reported-by: Hajo Noerenberg <hajo-linux-bugzilla@xxxxxxxxxxxxx>
> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> Reviewed-by: Marek Behún <kabel@xxxxxxxxxx>
> [ refactored a little ]
> Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
But this patch looks good to me. I hope that we should be able to use this patch
as a precedent for other drivers as well.
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
- Mani
> ---
> This was discussed back in 2022
> https://lore.kernel.org/linux-arm-kernel/20220709161858.15031-1-pali@xxxxxxxxxx/
> IMO Pali gave good arguments about why it should be applied, and Lorenzo
> agreed.
>
> Can we get this applied?
> ---
> drivers/pci/controller/pci-mvebu.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index 29fe09c99e7d..91a02b23aeb1 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -1683,8 +1683,15 @@ static void mvebu_pcie_remove(struct platform_device *pdev)
> irq_set_chained_handler_and_data(irq, NULL, NULL);
>
> /* Remove IRQ domains. */
> - if (port->intx_irq_domain)
> + if (port->intx_irq_domain) {
> + for (int j = 0; j < PCI_NUM_INTX; j++) {
> + int virq = irq_find_mapping(port->intx_irq_domain, j);
> +
> + if (virq > 0)
> + irq_dispose_mapping(virq);
> + }
> irq_domain_remove(port->intx_irq_domain);
> + }
>
> /* Free config space for emulated root bridge. */
> pci_bridge_emul_cleanup(&port->bridge);
> --
> 2.44.2
>
>
--
மணிவண்ணன் சதாசிவம்
