* [PATCH 0/1] hw/intc/aspeed: Fix IRQ handler mask check @ 2025-03-20 9:25 Steven Lee via 2025-03-20 9:25 ` [PATCH 1/1] " Steven Lee via 0 siblings, 1 reply; 5+ messages in thread From: Steven Lee via @ 2025-03-20 9:25 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, longzl2, yunlin.tang Dear maintainers, This patch addresses an issue in the ast27x0 interrupt controller where the IRQ handler mask check does not correctly apply the select variable. The fix ensures that the interrupt service routine (ISR) is triggered appropriately for interrupts within the same IRQ group. Please help to review this patch at your convenience. Best regards, Steven Lee Steven Lee (1): hw/intc/aspeed: Fix IRQ handler mask check hw/intc/aspeed_intc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check 2025-03-20 9:25 [PATCH 0/1] hw/intc/aspeed: Fix IRQ handler mask check Steven Lee via @ 2025-03-20 9:25 ` Steven Lee via 2025-03-20 15:29 ` Cédric Le Goater 0 siblings, 1 reply; 5+ messages in thread From: Steven Lee via @ 2025-03-20 9:25 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, longzl2, yunlin.tang Updated the IRQ handler mask check to AND with select variable. This ensures that the interrupt service routine is correctly triggered for the interrupts within the same irq group. For example, both `eth0` and the debug UART are handled in `GICINT132`. Without this fix, the debug console may hang if the `eth0` ISR is not handled. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> Change-Id: Ic3609eb72218dfd68be6057d78b8953b18828709 --- hw/intc/aspeed_intc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index 3fd417084f..f17bf43925 100644 --- a/hw/intc/aspeed_intc.c +++ b/hw/intc/aspeed_intc.c @@ -111,7 +111,7 @@ static void aspeed_intc_set_irq_handler(AspeedINTCState *s, outpin_idx = intc_irq->outpin_idx; inpin_idx = intc_irq->inpin_idx; - if (s->mask[inpin_idx] || s->regs[status_reg]) { + if ((s->mask[inpin_idx] & select) || (s->regs[status_reg] & select)) { /* * a. mask is not 0 means in ISR mode * sources interrupt routine are executing. -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check 2025-03-20 9:25 ` [PATCH 1/1] " Steven Lee via @ 2025-03-20 15:29 ` Cédric Le Goater 2025-03-21 2:54 ` Steven Lee 0 siblings, 1 reply; 5+ messages in thread From: Cédric Le Goater @ 2025-03-20 15:29 UTC (permalink / raw) To: Steven Lee, Peter Maydell, Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, longzl2, yunlin.tang Hello Steven, On 3/20/25 10:25, Steven Lee wrote: > Updated the IRQ handler mask check to AND with select variable. > This ensures that the interrupt service routine is correctly triggered > for the interrupts within the same irq group. > > For example, both `eth0` and the debug UART are handled in `GICINT132`. > Without this fix, the debug console may hang if the `eth0` ISR is not > handled. > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > Change-Id: Ic3609eb72218dfd68be6057d78b8953b18828709 > --- I think the issue was introduced by the initial commit : d831c5fd8682 ("aspeed/intc: Add AST2700 support") Is that correct ? Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > hw/intc/aspeed_intc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c > index 3fd417084f..f17bf43925 100644 > --- a/hw/intc/aspeed_intc.c > +++ b/hw/intc/aspeed_intc.c > @@ -111,7 +111,7 @@ static void aspeed_intc_set_irq_handler(AspeedINTCState *s, > outpin_idx = intc_irq->outpin_idx; > inpin_idx = intc_irq->inpin_idx; > > - if (s->mask[inpin_idx] || s->regs[status_reg]) { > + if ((s->mask[inpin_idx] & select) || (s->regs[status_reg] & select)) { > /* > * a. mask is not 0 means in ISR mode > * sources interrupt routine are executing. ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check 2025-03-20 15:29 ` Cédric Le Goater @ 2025-03-21 2:54 ` Steven Lee 2025-03-21 6:28 ` Cédric Le Goater 0 siblings, 1 reply; 5+ messages in thread From: Steven Lee @ 2025-03-21 2:54 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Troy Lee, longzl2@lenovo.com, Yunlin Tang Hi Cédric, > -----Original Message----- > From: Cédric Le Goater <clg@kaod.org> > Sent: Thursday, March 20, 2025 11:29 PM > To: Steven Lee <steven_lee@aspeedtech.com>; Peter Maydell > <peter.maydell@linaro.org>; Troy Lee <leetroy@gmail.com>; Jamin Lin > <jamin_lin@aspeedtech.com>; Andrew Jeffery > <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open > list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here > <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_lee@aspeedtech.com>; longzl2@lenovo.com; Yunlin Tang > <yunlin.tang@aspeedtech.com> > Subject: Re: [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check > > Hello Steven, > > On 3/20/25 10:25, Steven Lee wrote: > > Updated the IRQ handler mask check to AND with select variable. > > This ensures that the interrupt service routine is correctly triggered > > for the interrupts within the same irq group. > > > > For example, both `eth0` and the debug UART are handled in `GICINT132`. > > Without this fix, the debug console may hang if the `eth0` ISR is not > > handled. > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > Change-Id: Ic3609eb72218dfd68be6057d78b8953b18828709 > > --- > > I think the issue was introduced by the initial commit : > > d831c5fd8682 ("aspeed/intc: Add AST2700 support") > > Is that correct ? > Yes, and the implementation is based on commit 38ba38d (hw/intc/aspeed: Add Support for AST2700 INTCIO Controller). Additionally, I sent a patch series for AST27x0 multi-SoC support on March 13th. However, I forgot to append the v2 tag to the series. Should I resend it with the correct version tag? Patchwork link: https://patchwork.kernel.org/project/qemu-devel/list/?series=943379 Thanks, Steven > Reviewed-by: Cédric Le Goater <clg@redhat.com> > > Thanks, > > C. > > > > hw/intc/aspeed_intc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index > > 3fd417084f..f17bf43925 100644 > > --- a/hw/intc/aspeed_intc.c > > +++ b/hw/intc/aspeed_intc.c > > @@ -111,7 +111,7 @@ static void > aspeed_intc_set_irq_handler(AspeedINTCState *s, > > outpin_idx = intc_irq->outpin_idx; > > inpin_idx = intc_irq->inpin_idx; > > > > - if (s->mask[inpin_idx] || s->regs[status_reg]) { > > + if ((s->mask[inpin_idx] & select) || (s->regs[status_reg] & > > + select)) { > > /* > > * a. mask is not 0 means in ISR mode > > * sources interrupt routine are executing. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check 2025-03-21 2:54 ` Steven Lee @ 2025-03-21 6:28 ` Cédric Le Goater 0 siblings, 0 replies; 5+ messages in thread From: Cédric Le Goater @ 2025-03-21 6:28 UTC (permalink / raw) To: Steven Lee, Peter Maydell, Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Troy Lee, longzl2@lenovo.com, Yunlin Tang Hello Steven, On 3/21/25 03:54, Steven Lee wrote: > Hi Cédric, > >> -----Original Message----- >> From: Cédric Le Goater <clg@kaod.org> >> Sent: Thursday, March 20, 2025 11:29 PM >> To: Steven Lee <steven_lee@aspeedtech.com>; Peter Maydell >> <peter.maydell@linaro.org>; Troy Lee <leetroy@gmail.com>; Jamin Lin >> <jamin_lin@aspeedtech.com>; Andrew Jeffery >> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open >> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here >> <qemu-devel@nongnu.org> >> Cc: Troy Lee <troy_lee@aspeedtech.com>; longzl2@lenovo.com; Yunlin Tang >> <yunlin.tang@aspeedtech.com> >> Subject: Re: [PATCH 1/1] hw/intc/aspeed: Fix IRQ handler mask check >> >> Hello Steven, >> >> On 3/20/25 10:25, Steven Lee wrote: >>> Updated the IRQ handler mask check to AND with select variable. >>> This ensures that the interrupt service routine is correctly triggered >>> for the interrupts within the same irq group. >>> >>> For example, both `eth0` and the debug UART are handled in `GICINT132`. >>> Without this fix, the debug console may hang if the `eth0` ISR is not >>> handled. >>> >>> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> >>> Change-Id: Ic3609eb72218dfd68be6057d78b8953b18828709 >>> --- >> >> I think the issue was introduced by the initial commit : >> >> d831c5fd8682 ("aspeed/intc: Add AST2700 support") >> >> Is that correct ? >> > > Yes, and the implementation is based on commit 38ba38d (hw/intc/aspeed: Add Support for AST2700 INTCIO Controller). > > Additionally, I sent a patch series for AST27x0 multi-SoC support on March 13th. However, I forgot to append the v2 tag to the series. That's OK. We are in -rc phase of the QEMU 10.0 cycle, so I am looking at fixes first. > Should I resend it with the correct version tag? No need. This is enough: Fixes: d831c5fd8682 ("aspeed/intc: Add AST2700 support") Thanks, C. > Patchwork link: > https://patchwork.kernel.org/project/qemu-devel/list/?series=943379 > > Thanks, > Steven > >> Reviewed-by: Cédric Le Goater <clg@redhat.com> >> >> Thanks, >> >> C. >> >> >>> hw/intc/aspeed_intc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index >>> 3fd417084f..f17bf43925 100644 >>> --- a/hw/intc/aspeed_intc.c >>> +++ b/hw/intc/aspeed_intc.c >>> @@ -111,7 +111,7 @@ static void >> aspeed_intc_set_irq_handler(AspeedINTCState *s, >>> outpin_idx = intc_irq->outpin_idx; >>> inpin_idx = intc_irq->inpin_idx; >>> >>> - if (s->mask[inpin_idx] || s->regs[status_reg]) { >>> + if ((s->mask[inpin_idx] & select) || (s->regs[status_reg] & >>> + select)) { >>> /* >>> * a. mask is not 0 means in ISR mode >>> * sources interrupt routine are executing. > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-21 6:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-20 9:25 [PATCH 0/1] hw/intc/aspeed: Fix IRQ handler mask check Steven Lee via 2025-03-20 9:25 ` [PATCH 1/1] " Steven Lee via 2025-03-20 15:29 ` Cédric Le Goater 2025-03-21 2:54 ` Steven Lee 2025-03-21 6:28 ` Cédric Le Goater
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).