Re: [PATCH] PCI: plda: Synchronize chained IRQs during deinitialization

[Ali Tariq <alitariq45892@gmail.com>]

On 7/14/26 10:58 PM, Bjorn Helgaas wrote:
> On Tue, Jul 14, 2026 at 10:43:45PM +0500, Ali Tariq wrote:
>> During driver unbind or probe failure teardown,
>> plda_pcie_irq_domain_deinit() unlinks the chained interrupt
>> handlers for the main, MSI, and INTx interrupts using
>> irq_set_chained_handler_and_data(). However, this function
>> only updates the handler pointers and does not wait for any
>> in-flight interrupt handlers running on other CPUs to finish.
>>
>> If a PCIe interrupt fires concurrently with the teardown
>> process, the handler could continue running on another CPU.
>> If the hardware clocks are disabled shortly after in
>> host_deinit(), the executing handler will attempt to read
>> un-clocked PCIe registers, triggering a fatal system bus
>> fault (external abort) or kernel panic.
>>
>> Add synchronize_irq() after clearing each chained handler to
>> guarantee that any executing handlers have fully completed
>> before proceeding with interrupt domain removal and hardware
>> deinitialization.
>>
>> Fixes: 76c911396807 ("PCI: plda: Add host init/deinit and map bus functions")
>> Signed-off-by: Ali Tariq <alitariq45892@gmail.com>
>> ---
>>   drivers/pci/controller/plda/pcie-plda-host.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
>> index f9a34f323ad8..f6759e255c75 100644
>> --- a/drivers/pci/controller/plda/pcie-plda-host.c
>> +++ b/drivers/pci/controller/plda/pcie-plda-host.c
>> @@ -560,8 +560,13 @@ EXPORT_SYMBOL_GPL(plda_pcie_setup_iomems);
>>   static void plda_pcie_irq_domain_deinit(struct plda_pcie_rp *pcie)
>>   {
>>   	irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
>> +	synchronize_irq(pcie->irq);
>> +
>>   	irq_set_chained_handler_and_data(pcie->msi_irq, NULL, NULL);
>> +	synchronize_irq(pcie->msi_irq);
>> +
>>   	irq_set_chained_handler_and_data(pcie->intx_irq, NULL, NULL);
>> +	synchronize_irq(pcie->intx_irq);
> 
> Several other drivers call irq_set_chained_handler_and_data(..., NULL)
> without synchronize_irq().  Do they need similar fixes?
> 
>>   	irq_domain_remove(pcie->msi.dev_domain);
>>   
>> -- 
>> 2.34.1
>>

Hi Bjorn,

Thanks for the question. I looked at the wider pattern across the tree,
and severity does seem to depend heavily on what happens immediately
after the teardown in each specific driver, so I don't think a single
change is safe to apply broadly. I'll follow up separately with more
detail on what I found there.

For this specific driver, though, I think my patch needs to be
withdrawn. Tracing through kernel/irq/manage.c, synchronize_irq()'s
wait relies on the IRQD_IRQ_INPROGRESS flag, which is set and cleared
by handle_irq_event() in the normal interrupt dispatch path. Chained
handlers bypass that path entirely (dispatch goes straight to
desc->handle_irq), so this flag is never touched for them. The
fallback in that case is to query the irqchip directly via
.irq_get_irqchip_state(), but none of this driver's irq_chip
structures implement that callback, so the fallback also does
nothing.

The practical effect is that synchronize_irq() here returns
immediately regardless of whether a chained handler is still
executing, so this patch doesn't actually close the race it was meant
to fix.

I'd like to withdraw this specific patch and rework it, likely with an 
explicit in-driver flag around the chained handlers rather than relying 
on synchronize_irq(), since I don't think .irq_get_irqchip_state() maps
cleanly onto this driver's handler structure either (the status
register gets acked partway through handling, before the rest of the
dispatch work is done).

Regards,
Ali