[PATCH 3/4] irqchip/sg2042-msi: Fix broken affinity setting

Inochi Amaoto posted 4 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH 3/4] irqchip/sg2042-msi: Fix broken affinity setting
Posted by Inochi Amaoto 1 month, 4 weeks ago
When using NVME on SG2044, the NVME always complains "I/O tag XXX
(XXX) QID XX timeout, completion polled", which is caused by the
broken handler of the sg2042-msi driver.

As PLIC driver can only setting affinity when enabling, the sg2042-msi
does not properly handled affinity setting previously and enable irq in
an unexpected executing path.

Since the PCI template domain supports irq_startup/irq_shutdown, set
irq_chip_[startup/shutdown]_parent for irq_startup/irq_shutdown. So
the irq can be started properly.

Fixes: e96b93a97c90 ("irqchip/sg2042-msi: Add the Sophgo SG2044 MSI interrupt controller")
Reported-by: Han Gao <rabenda.cn@gmail.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/irqchip/irq-sg2042-msi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-sg2042-msi.c b/drivers/irqchip/irq-sg2042-msi.c
index bcfddc51bc6a..2b7ee17232ab 100644
--- a/drivers/irqchip/irq-sg2042-msi.c
+++ b/drivers/irqchip/irq-sg2042-msi.c
@@ -85,6 +85,8 @@ static void sg2042_msi_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *m
 
 static const struct irq_chip sg2042_msi_middle_irq_chip = {
 	.name			= "SG2042 MSI",
+	.irq_startup		= irq_chip_startup_parent,
+	.irq_shutdown		= irq_chip_shutdown_parent,
 	.irq_ack		= sg2042_msi_irq_ack,
 	.irq_mask		= irq_chip_mask_parent,
 	.irq_unmask		= irq_chip_unmask_parent,
@@ -114,6 +116,8 @@ static void sg2044_msi_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *m
 
 static struct irq_chip sg2044_msi_middle_irq_chip = {
 	.name			= "SG2044 MSI",
+	.irq_startup		= irq_chip_startup_parent,
+	.irq_shutdown		= irq_chip_shutdown_parent,
 	.irq_ack		= sg2044_msi_irq_ack,
 	.irq_mask		= irq_chip_mask_parent,
 	.irq_unmask		= irq_chip_unmask_parent,
@@ -186,7 +190,9 @@ static const struct irq_domain_ops sg204x_msi_middle_domain_ops = {
 };
 
 #define SG2042_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
-				   MSI_FLAG_USE_DEF_CHIP_OPS)
+				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
+				   MSI_FLAG_PCI_MSI_MASK_PARENT |\
+				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)
 
 #define SG2042_MSI_FLAGS_SUPPORTED MSI_GENERIC_FLAGS_MASK
 
@@ -201,7 +207,9 @@ static const struct msi_parent_ops sg2042_msi_parent_ops = {
 };
 
 #define SG2044_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
-				   MSI_FLAG_USE_DEF_CHIP_OPS)
+				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
+				   MSI_FLAG_PCI_MSI_MASK_PARENT |\
+				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)
 
 #define SG2044_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK |	\
 				    MSI_FLAG_PCI_MSIX)
-- 
2.50.1
Re: [PATCH 3/4] irqchip/sg2042-msi: Fix broken affinity setting
Posted by Andy Shevchenko 1 month, 3 weeks ago
On Thu, Aug 07, 2025 at 07:23:24PM +0800, Inochi Amaoto wrote:
> When using NVME on SG2044, the NVME always complains "I/O tag XXX
> (XXX) QID XX timeout, completion polled", which is caused by the
> broken handler of the sg2042-msi driver.
> 
> As PLIC driver can only setting affinity when enabling, the sg2042-msi
> does not properly handled affinity setting previously and enable irq in
> an unexpected executing path.
> 
> Since the PCI template domain supports irq_startup/irq_shutdown, set
> irq_chip_[startup/shutdown]_parent for irq_startup/irq_shutdown. So
> the irq can be started properly.

> Fixes: e96b93a97c90 ("irqchip/sg2042-msi: Add the Sophgo SG2044 MSI interrupt controller")
> Reported-by: Han Gao <rabenda.cn@gmail.com>

Closes ?

> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>

...

>  #define SG2042_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
> -				   MSI_FLAG_USE_DEF_CHIP_OPS)
> +				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
> +				   MSI_FLAG_PCI_MSI_MASK_PARENT |\

Can we indent \ to be on the same column (using TABs)?

> +				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)

...

>  #define SG2044_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
> -				   MSI_FLAG_USE_DEF_CHIP_OPS)
> +				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
> +				   MSI_FLAG_PCI_MSI_MASK_PARENT |\
> +				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)

Ditto.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 3/4] irqchip/sg2042-msi: Fix broken affinity setting
Posted by Inochi Amaoto 1 month, 3 weeks ago
On Mon, Aug 11, 2025 at 04:43:00PM +0200, Andy Shevchenko wrote:
> On Thu, Aug 07, 2025 at 07:23:24PM +0800, Inochi Amaoto wrote:
> > When using NVME on SG2044, the NVME always complains "I/O tag XXX
> > (XXX) QID XX timeout, completion polled", which is caused by the
> > broken handler of the sg2042-msi driver.
> > 
> > As PLIC driver can only setting affinity when enabling, the sg2042-msi
> > does not properly handled affinity setting previously and enable irq in
> > an unexpected executing path.
> > 
> > Since the PCI template domain supports irq_startup/irq_shutdown, set
> > irq_chip_[startup/shutdown]_parent for irq_startup/irq_shutdown. So
> > the irq can be started properly.
> 
> > Fixes: e96b93a97c90 ("irqchip/sg2042-msi: Add the Sophgo SG2044 MSI interrupt controller")
> > Reported-by: Han Gao <rabenda.cn@gmail.com>
> 
> Closes ?
> 

I got a direct private email from Han, so I think there is no pulic
Closes.

> > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> 
> ...
> 
> >  #define SG2042_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
> > -				   MSI_FLAG_USE_DEF_CHIP_OPS)
> > +				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
> > +				   MSI_FLAG_PCI_MSI_MASK_PARENT |\
> 
> Can we indent \ to be on the same column (using TABs)?
> 

Yeah, of course

> > +				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)
> 
> ...
> 
> >  #define SG2044_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |	\
> > -				   MSI_FLAG_USE_DEF_CHIP_OPS)
> > +				   MSI_FLAG_USE_DEF_CHIP_OPS |	\
> > +				   MSI_FLAG_PCI_MSI_MASK_PARENT |\
> > +				   MSI_FLAG_PCI_MSI_STARTUP_PARENT)
> 
> Ditto.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
Re: [PATCH 3/4] irqchip/sg2042-msi: Fix broken affinity setting
Posted by Andy Shevchenko 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 06:37:35AM +0800, Inochi Amaoto wrote:
> On Mon, Aug 11, 2025 at 04:43:00PM +0200, Andy Shevchenko wrote:
> > On Thu, Aug 07, 2025 at 07:23:24PM +0800, Inochi Amaoto wrote:
> > > When using NVME on SG2044, the NVME always complains "I/O tag XXX
> > > (XXX) QID XX timeout, completion polled", which is caused by the
> > > broken handler of the sg2042-msi driver.
> > > 
> > > As PLIC driver can only setting affinity when enabling, the sg2042-msi
> > > does not properly handled affinity setting previously and enable irq in
> > > an unexpected executing path.
> > > 
> > > Since the PCI template domain supports irq_startup/irq_shutdown, set
> > > irq_chip_[startup/shutdown]_parent for irq_startup/irq_shutdown. So
> > > the irq can be started properly.
> > 
> > > Fixes: e96b93a97c90 ("irqchip/sg2042-msi: Add the Sophgo SG2044 MSI interrupt controller")
> > > Reported-by: Han Gao <rabenda.cn@gmail.com>
> > 
> > Closes ?
> 
> I got a direct private email from Han, so I think there is no pulic
> Closes.

It's better to follow current practise (by at least some of the fix code
authors) to make this as a comment in the patch (text, that goes after '---'
but before actual diff).

No need to resent just for that, as you basically explained it here.
(but it seems we will have a new version anyway)

-- 
With Best Regards,
Andy Shevchenko