Yes you are right. I spoke with netbsd devs about this. On arm at least, they enable interrupts before calling the handler but ack them in a different order to calling the handler, depending on trigger mode. That is, for edge triggered, the eoi comes before the handler so they don't miss interrupts but for level triggered, the eoi comes after the handler since a high interrupt line doesn't keep triggering until its been eoi'd so makes sense to handle those first.
So I think we still need to change the current code but make two strategies, one for each trigger mode. Damien Sent from Proton Mail Android -------- Original Message -------- On 17/7/25 7:56 pm, Samuel Thibault <samuel.thiba...@gnu.org> wrote: > Hello, > > Please check the history of this file. There has been a lot of pushing > one way and another, we need to make sure what is exactly right. > Notably, 362c84a08a1b8f1eb7f9c1c37c6ed7cece348ee4 explicitly made it > ack before calling the handler. Please check the rationale behind it, > otherwise we will just re-open bugs that were previously fixed and we > are not actually making progress. > > Samuel > > Damien Zammit, le jeu. 17 juil. 2025 09:20:44 +0000, a ecrit: > > This should ensure we don't ack nested interrupts out of order. > > > > TESTED: On SMP this stops failure in xhci from hanging rumpnet. > > > > When a second usb driver is executed, the second one detects a problem > > and disables the uhub0 port, which breaks the first usb driver. > > But this does not block rumpnet from serving packets, sharing irq 11. > > The usb stack can be recovered by killing both usb drivers and > > restarting one. Before this change, the behaviour was that irq 11 > > became stuck and unusable for any device. > > > > Parallel compiling speed is greatly improved with this commit on SMP. > > > > UP+apic still works as per usual. > > --- > > i386/i386at/interrupt.S | 34 +++++++++++++++++----------------- > > x86_64/interrupt.S | 32 ++++++++++++++++---------------- > > 2 files changed, 33 insertions(+), 33 deletions(-) > > > > diff --git a/i386/i386at/interrupt.S b/i386/i386at/interrupt.S > > index 77424b43..0775eaf1 100644 > > --- a/i386/i386at/interrupt.S > > +++ b/i386/i386at/interrupt.S > > @@ -61,6 +61,23 @@ ENTRY(interrupt) > > je _call_local_ast > > #endif > > > > + movl S_IPL,%eax > > + movl %eax,4(%esp) /* previous ipl as 2nd arg */ > > + > > + movl S_RET,%eax > > + movl %eax,8(%esp) /* return address as 3rd arg */ > > + > > + movl S_REGS,%eax > > + movl %eax,12(%esp) /* address of interrupted registers as > 4th arg */ > > + > > + movl S_IRQ,%eax /* copy irq number */ > > + > > + shll $2,%eax /* irq * 4 */ > > + movl EXT(iunit)(%eax),%edx /* get device unit number */ > > + movl %edx,(%esp) /* unit number as 1st arg */ > > + > > + call *EXT(ivect)(%eax) /* call interrupt handler */ > > + > > #ifndef APIC > > movl $1,%eax > > shll %cl,%eax /* get corresponding IRQ mask */ > > @@ -102,23 +119,6 @@ ENTRY(interrupt) > > call EXT(ioapic_irq_eoi) /* ioapic irq specific EOI */ > > #endif > > > > - movl S_IPL,%eax > > - movl %eax,4(%esp) /* previous ipl as 2nd arg */ > > - > > - movl S_RET,%eax > > - movl %eax,8(%esp) /* return address as 3rd arg */ > > - > > - movl S_REGS,%eax > > - movl %eax,12(%esp) /* address of interrupted registers as > 4th arg */ > > - > > - movl S_IRQ,%eax /* copy irq number */ > > - > > - shll $2,%eax /* irq * 4 */ > > - movl EXT(iunit)(%eax),%edx /* get device unit number */ > > - movl %edx,(%esp) /* unit number as 1st arg */ > > - > > - call *EXT(ivect)(%eax) /* call interrupt handler */ > > - > > _completed: > > movl S_IPL,%eax /* restore previous ipl */ > > movl %eax,(%esp) > > diff --git a/x86_64/interrupt.S b/x86_64/interrupt.S > > index 6fb77727..870c7fde 100644 > > --- a/x86_64/interrupt.S > > +++ b/x86_64/interrupt.S > > @@ -61,6 +61,22 @@ ENTRY(interrupt) > > je _call_local_ast > > #endif > > > > + ; > > + movq S_IPL,S_ARG1 /* previous ipl as 2nd arg */ > > + > > + ; > > + movq S_RET,S_ARG2 /* return address as 3th arg */ > > + > > + ; > > + movq S_REGS,S_ARG3 /* address of interrupted registers as > 4th arg */ > > + > > + movl S_IRQ,%eax /* copy irq number */ > > + shll $2,%eax /* irq * 4 */ > > + movl EXT(iunit)(%rax),%edi /* get device unit number as 1st arg */ > > + > > + shll $1,%eax /* irq * 8 */ > > + call *EXT(ivect)(%rax) /* call interrupt handler */ > > + > > #ifndef APIC > > movl $1,%eax > > shll %cl,%eax /* get corresponding IRQ mask */ > > @@ -102,22 +118,6 @@ ENTRY(interrupt) > > call EXT(ioapic_irq_eoi) /* ioapic irq specific EOI */ > > #endif > > > > - ; > > - movq S_IPL,S_ARG1 /* previous ipl as 2nd arg */ > > - > > - ; > > - movq S_RET,S_ARG2 /* return address as 3th arg */ > > - > > - ; > > - movq S_REGS,S_ARG3 /* address of interrupted registers as > 4th arg */ > > - > > - movl S_IRQ,%eax /* copy irq number */ > > - shll $2,%eax /* irq * 4 */ > > - movl EXT(iunit)(%rax),%edi /* get device unit number as 1st arg */ > > - > > - shll $1,%eax /* irq * 8 */ > > - call *EXT(ivect)(%rax) /* call interrupt handler */ > > - > > _completed: > > movl S_IPL,%edi /* restore previous ipl */ > > call splx_cli /* restore previous ipl */ > > -- > > 2.45.2 > > > > > > > > -- > Samuel > <y> ça gaze ? > <l> prout > -+- #ens-mim - ouvrez les fenêtres ! -+- >
