[PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()

Inochi Amaoto posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/pci/msi/irqdomain.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Inochi Amaoto 1 month, 1 week ago
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
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Mark Brown 1 month ago
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.
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Bjorn Helgaas 1 month ago
[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
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Bjorn Helgaas 1 month ago
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
[PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Naresh Kamboju 1 month ago
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
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Bjorn Helgaas 1 month ago
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
>
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Inochi Amaoto 1 month ago
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
> >
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Bjorn Helgaas 1 month ago
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.
Re: [PATCH] PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in cond_[startup|shutdown]_parent()
Posted by Jon Hunter 1 month, 1 week ago
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