drivers/pci/msi/irqdomain.c | 5 +++++ 1 file changed, 5 insertions(+)
For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT,
the newly added callback irq_startup() and irq_shutdown() for
pci_msi[x]_templete will not unmask/mask the interrupt when startup/
shutdown the interrupt. This will prevent the interrupt from being
enabled/disabled normally.
Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the
cond_[startup|shutdown]_parent(). So the interrupt can be normally
unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT.
Fixes: 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown for per device domains")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Closes: https://lore.kernel.org/regressions/aK4O7Hl8NCVEMznB@monster/
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/pci/msi/irqdomain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
index e0a800f918e8..b11b7f63f0d6 100644
--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -154,6 +154,8 @@ static void cond_shutdown_parent(struct irq_data *data)
if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT))
irq_chip_shutdown_parent(data);
+ else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT))
+ irq_chip_mask_parent(data);
}
static unsigned int cond_startup_parent(struct irq_data *data)
@@ -162,6 +164,9 @@ static unsigned int cond_startup_parent(struct irq_data *data)
if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT))
return irq_chip_startup_parent(data);
+ else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT))
+ irq_chip_unmask_parent(data);
+
return 0;
}
--
2.51.0
On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > the newly added callback irq_startup() and irq_shutdown() for > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > shutdown the interrupt. This will prevent the interrupt from being > enabled/disabled normally. > > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > cond_[startup|shutdown]_parent(). So the interrupt can be normally > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. Tested-by: Mark Brown <broonie@kernel.org> This is causing multiple platforms to fail to boot in -next, it'd be great if we could get it merged to fix them.
[cc->to: Thomas] On Mon, Sep 01, 2025 at 01:35:17PM +0100, Mark Brown wrote: > On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > > the newly added callback irq_startup() and irq_shutdown() for > > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > > shutdown the interrupt. This will prevent the interrupt from being > > enabled/disabled normally. > > > > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > > cond_[startup|shutdown]_parent(). So the interrupt can be normally > > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. > > Tested-by: Mark Brown <broonie@kernel.org> > > This is causing multiple platforms to fail to boot in -next, it'd be > great if we could get it merged to fix them. It looks like you merged 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown for per device domains"), Thomas. I assume you'll merge this fix too? Let me know if you'd prefer that I take it. Looks like a candidate for v6.17. Bjorn
On Mon, Sep 01, 2025 at 10:24:54AM -0500, Bjorn Helgaas wrote: > [cc->to: Thomas] > > On Mon, Sep 01, 2025 at 01:35:17PM +0100, Mark Brown wrote: > > On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > > > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > > > the newly added callback irq_startup() and irq_shutdown() for > > > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > > > shutdown the interrupt. This will prevent the interrupt from being > > > enabled/disabled normally. > > > > > > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > > > cond_[startup|shutdown]_parent(). So the interrupt can be normally > > > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. > > > > Tested-by: Mark Brown <broonie@kernel.org> > > > > This is causing multiple platforms to fail to boot in -next, it'd be > > great if we could get it merged to fix them. > > It looks like you merged 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown > for per device domains"), Thomas. I assume you'll merge this fix too? > > Let me know if you'd prefer that I take it. Looks like a candidate > for v6.17. Oops, looks like there's a v2: https://lore.kernel.org/r/20250827230943.17829-1-inochiama@gmail.com
On Wed, 27 Aug 2025 at 06:17, Nathan Chancellor <nathan@kernel.org> wrote: > > On Wed, Aug 27, 2025 at 07:28:46AM +0800, Inochi Amaoto wrote: > > OK, I guess I know why: I have missed one condition for startup. > > > > Could you test the following patch? If worked, I will send it as > > a fix. > > Yes, that appears to resolve the issue on one system. I cannot test the > other at the moment since it is under load. I have built on top of Linux next-20250826 tag and the qemu-arm64 boot test pass and LTP smoke test also pass. > > Tested-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> -- Linaro LKFT https://lkft.linaro.org
On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > the newly added callback irq_startup() and irq_shutdown() for > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > shutdown the interrupt. This will prevent the interrupt from being > enabled/disabled normally. s/templete/template/ AFAICS cond_startup_parent() is used by pci_irq_startup_msi() and pci_irq_startup_msix() in pci_msi_template; cond_shutdown_parent() is used by pci_irq_shutdown_msi() and pci_irq_shutdown_msix() in pci_msix_template. > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > cond_[startup|shutdown]_parent(). So the interrupt can be normally > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. > > Fixes: 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown for per device domains") > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > Closes: https://lore.kernel.org/regressions/aK4O7Hl8NCVEMznB@monster/ I guess there are several other reporters and reports that could be added here. Up to you whether to add them. > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> > Tested-by: Nathan Chancellor <nathan@kernel.org> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/msi/irqdomain.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c > index e0a800f918e8..b11b7f63f0d6 100644 > --- a/drivers/pci/msi/irqdomain.c > +++ b/drivers/pci/msi/irqdomain.c > @@ -154,6 +154,8 @@ static void cond_shutdown_parent(struct irq_data *data) > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > irq_chip_shutdown_parent(data); > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > + irq_chip_mask_parent(data); > } > > static unsigned int cond_startup_parent(struct irq_data *data) > @@ -162,6 +164,9 @@ static unsigned int cond_startup_parent(struct irq_data *data) > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > return irq_chip_startup_parent(data); > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > + irq_chip_unmask_parent(data); > + > return 0; > } > > -- > 2.51.0 >
On Wed, Aug 27, 2025 at 01:32:02PM -0500, Bjorn Helgaas wrote: > On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > > the newly added callback irq_startup() and irq_shutdown() for > > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > > shutdown the interrupt. This will prevent the interrupt from being > > enabled/disabled normally. > > s/templete/template/ > > AFAICS cond_startup_parent() is used by pci_irq_startup_msi() and > pci_irq_startup_msix() in pci_msi_template; cond_shutdown_parent() is > used by pci_irq_shutdown_msi() and pci_irq_shutdown_msix() in > pci_msix_template. > cond_startup_parent() is used by pci_irq_startup_msi() in pci_msi_template and pci_irq_startup_msix() in pci_msix_template; cond_shutdown_parent() is used by pci_irq_shutdown_msi() in pci_msi_template and pci_irq_shutdown_msix() in pci_msix_template. > > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > > cond_[startup|shutdown]_parent(). So the interrupt can be normally > > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. > > > > Fixes: 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown for per device domains") > > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > > Closes: https://lore.kernel.org/regressions/aK4O7Hl8NCVEMznB@monster/ > > I guess there are several other reporters and reports that could be > added here. Up to you whether to add them. > Yeah, there are some missing tag from the original post, as this patch is already submitted. I will add it in v2. > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > --- > > drivers/pci/msi/irqdomain.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c > > index e0a800f918e8..b11b7f63f0d6 100644 > > --- a/drivers/pci/msi/irqdomain.c > > +++ b/drivers/pci/msi/irqdomain.c > > @@ -154,6 +154,8 @@ static void cond_shutdown_parent(struct irq_data *data) > > > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > > irq_chip_shutdown_parent(data); > > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > > + irq_chip_mask_parent(data); > > } > > > > static unsigned int cond_startup_parent(struct irq_data *data) > > @@ -162,6 +164,9 @@ static unsigned int cond_startup_parent(struct irq_data *data) > > > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > > return irq_chip_startup_parent(data); > > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > > + irq_chip_unmask_parent(data); > > + > > return 0; > > } > > > > -- > > 2.51.0 > >
On Thu, Aug 28, 2025 at 06:52:39AM +0800, Inochi Amaoto wrote: > On Wed, Aug 27, 2025 at 01:32:02PM -0500, Bjorn Helgaas wrote: > > On Wed, Aug 27, 2025 at 02:29:07PM +0800, Inochi Amaoto wrote: > > > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > > > the newly added callback irq_startup() and irq_shutdown() for > > > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > > > shutdown the interrupt. This will prevent the interrupt from being > > > enabled/disabled normally. > > > > s/templete/template/ > > > AFAICS cond_startup_parent() is used by pci_irq_startup_msi() and > > pci_irq_startup_msix() in pci_msi_template; cond_shutdown_parent() is > > used by pci_irq_shutdown_msi() and pci_irq_shutdown_msix() in > > pci_msix_template. > > cond_startup_parent() is used by pci_irq_startup_msi() in > pci_msi_template and pci_irq_startup_msix() in pci_msix_template; > cond_shutdown_parent() is used by pci_irq_shutdown_msi() in > pci_msi_template and pci_irq_shutdown_msix() in pci_msix_template. Right, I really screwed that up when I noticed the "*_template" structure names and added them to my description.
On 27/08/2025 07:29, Inochi Amaoto wrote: > For msi controller that only supports MSI_FLAG_PCI_MSI_MASK_PARENT, > the newly added callback irq_startup() and irq_shutdown() for > pci_msi[x]_templete will not unmask/mask the interrupt when startup/ > shutdown the interrupt. This will prevent the interrupt from being > enabled/disabled normally. > > Add the missing check for MSI_FLAG_PCI_MSI_MASK_PARENT in the > cond_[startup|shutdown]_parent(). So the interrupt can be normally > unmasked/masked if it does not support MSI_FLAG_PCI_MSI_MASK_PARENT. > > Fixes: 54f45a30c0d0 ("PCI/MSI: Add startup/shutdown for per device domains") > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > Closes: https://lore.kernel.org/regressions/aK4O7Hl8NCVEMznB@monster/ > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> > Tested-by: Nathan Chancellor <nathan@kernel.org> > --- > drivers/pci/msi/irqdomain.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c > index e0a800f918e8..b11b7f63f0d6 100644 > --- a/drivers/pci/msi/irqdomain.c > +++ b/drivers/pci/msi/irqdomain.c > @@ -154,6 +154,8 @@ static void cond_shutdown_parent(struct irq_data *data) > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > irq_chip_shutdown_parent(data); > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > + irq_chip_mask_parent(data); > } > > static unsigned int cond_startup_parent(struct irq_data *data) > @@ -162,6 +164,9 @@ static unsigned int cond_startup_parent(struct irq_data *data) > > if (unlikely(info->flags & MSI_FLAG_PCI_MSI_STARTUP_PARENT)) > return irq_chip_startup_parent(data); > + else if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT)) > + irq_chip_unmask_parent(data); > + > return 0; > } FWIW, this also fixes a boot regression for Tegra124 Jetson TK1. Tested-by: Jon Hunter <jonathanh@nvidia.com> Thanks! Jon -- nvpublic
© 2016 - 2025 Red Hat, Inc.