From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr2-f0.google.com (mail-wr2-f0.google.com [74.125.225.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D5B7379C47 for ; Thu, 16 Jul 2026 09:25:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.225.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784193924; cv=none; b=phTDfzVoEghlw3MlX+uLHcGOFK8mwUoF7O65SuizeeUb7HNyWSaZHQf/Ds4JDhVPcnzq2txY+cNp6KYeD3sw5lrHPpj7wTw6qIhZQDTLxa16GX1AVb0rc7t1kOz2mIqpaJsB1uxw2f4WdNS3bCfhJX5LrCvn2FVVRGgJf+GK/JU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784193924; c=relaxed/simple; bh=Ka+MiYwZXO9ZZk3ZdrXKIr03QaEUVx8pkcBqrszIYCo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GJPPqi8QvRKcaFSwLELt0nQvQRlyBq1XnbO6nzwkKyUrEvkq8OsOgorNafn8gsFOvFmO6kAtP0XbjOj8xzHr5wVdK+y3BDweZ9ve1WIxt5s0cR66+cH8X4jqohYy6MUaeRGAkwsMFbEnyoe/TZh3ggz4TWd3J4Zs/GW8r7F1JjQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=DAPuNffc; arc=none smtp.client-ip=74.125.225.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DAPuNffc" Received: by mail-wr2-f0.google.com with SMTP id ffacd0b85a97d-47528970fbdso1309722f8f.1 for ; Thu, 16 Jul 2026 02:25:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784193921; x=1784798721; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=y2umLa0OP6cXw7OYRebqCIfZ0tp2RQ5ZMbVPQ4YiGsA=; b=DAPuNffcfoaREG4/yWXNtcDLU4eZ7rJyEXimYKLxR2MFv78E9RRdtPmlmuPPGzYUSS YcVmkMajOcyRJJL1B5hINKLf/FdGjMyVfyPiOULAC3ME+F6NN8ez6PcHGgbdvPkCwqp0 fDnB4lUfRi3iULGL9zWSMY8NV4Nx7BRUgvV1vrQblRmTtxrnA3Xdqngal7GFZbXggj2+ ZdYgZb+qgU+1OLErKPFZ3p2nNk6ckLsQBwS1/rjKoCwltvXWngz3tG0Q+TOusGct5AIG TRr4y8XnfoTKvkjG+4HvU06pLyulm7Yn15D8Vgt6MuYLq399552RAGN/3OFWRf2u+9Kq nm3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784193921; x=1784798721; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=y2umLa0OP6cXw7OYRebqCIfZ0tp2RQ5ZMbVPQ4YiGsA=; b=Z1exrRnGQiZC6ai2UAjdsunc3yViqwzea06tWJv1Zbnge1cqJuPYID7Y7WMyN37Ppi Ff/BG4Fz3RlGpbWc2CZ2A5xeAUpDg5w18TBDwyNFFAGloaekVjpWGjq8sfA2CM7MlofE slxjjimBrXi8C4io8f0dE3OWgjnFrKiNgAcpozxb2T2KXWvvcRbbpes6SgOJOmnk9kKN NB7TOb/Vjmp6bjiaBYFmP6WR8v6TGE35f+jphdrGtFRx/EBCgkRtuiN0J16VQLOtfygJ Ir3kYjpupIzfzAIBTk68tD/NJJ923qMzWCWue1HEqdT3pdahjd5uFI4HzObD4K7E86p8 M6Dw== X-Forwarded-Encrypted: i=1; AHgh+Rqhfed3Bn6EvxBxx4tCO74w4NPCjYE7P0tcyrtyVeJgoMh8QzJN9RJvJyc6EkN/4wzXPyWAnHwq8KoWIHU=@vger.kernel.org X-Gm-Message-State: AOJu0Yxh+wE7OQ9tpBxpJGnXOVxsftakqwrW9ILnciVITLmWcZJ/1n+r QpRmB1WYf6Cs6QPrmx1yq3M+fA0/OZJI26TLcX0rvAePTxPQbJZGOUvQ X-Gm-Gg: AfdE7cniF4xgYh92EtTubmUzcO5oraHJDDeUSmXLKXiLNTh+jKqIWcvYzqQbsSjCaeH 6LTWJVrG7a8bM657ZI/a++AqioFlGjbTX5r9La97rHqCrKZFE+l4Z2/TJlhIinlOHzO2DBvmzcc 2LDjuY7Zh781njf9dqw4tBYeppl5meqXFHmv5ysIXnVdlHE3lcgXcUmAPRQO3VpQgV0xqsLkTGf 4liStlyTPSbpRq73I1BWt77R3aAt4gzfYYEk+cTT1oYk4bxJNVBVADR8A10EAsm7nODU10mz17s Vq+KS3ZR3poCmJ3ud4SX0Eh34oR6Ls9EUCv+1jr7ExnoppWIva2hzmLyaepSu679mizHD5XHBG4 wlClRz7p7/+qzgM57q9ma2QXHl1muIy+63xtkV50lKPQC5Ux246YpAsr4gp/RrqGtQc3ijjD8uc WqAvqobWl5mIz+7v5hAvPRPf4= X-Received: by 2002:a05:6000:26c7:b0:46d:55a5:8ec5 with SMTP id ffacd0b85a97d-47f2dcf9e3emr22598452f8f.33.1784193920312; Thu, 16 Jul 2026 02:25:20 -0700 (PDT) Received: from [192.168.101.113] ([110.93.227.81]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f4635082csm22937744f8f.7.2026.07.16.02.25.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Jul 2026 02:25:19 -0700 (PDT) Message-ID: <0b5cc400-1680-4f2e-ba41-5f391fcf979e@gmail.com> Date: Thu, 16 Jul 2026 14:25:16 +0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] PCI: plda: Synchronize chained IRQs during deinitialization To: Bjorn Helgaas Cc: Daire McNamara , Lorenzo Pieralisi , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , Manivannan Sadhasivam , Rob Herring , Bjorn Helgaas , Mason Huo , Minda Chen , "open list:PCI DRIVER FOR PLDA PCIE IP" , open list References: <20260714175852.GA1387886@bhelgaas> Content-Language: en-US From: Ali Tariq In-Reply-To: <20260714175852.GA1387886@bhelgaas> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 >> --- >> 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