Re: [ros-dev] [ros-diffs] [tfaber] 71307: [HAL] - Create a wrapper version of HalpEndSoftwareInterrupt that frees its stack before calling the noreturn version of the next software interrupt handler. Fixes excessive stack u...

[Alex Ionescu]

Wait, are you saying that Timo didn't discover this obvious bug for years?
Best regards,
Alex Ionescu

On Mon, May 16, 2016 at 4:06 PM, Timo Kreuzer [e-mail] wrote:
> Wait, are you suggesting that "doing everything the same as MS does" and
> "I don't need to care for C standards, I have a huge epeen!" and
> "OnlyOnePersonCanWriteHalCode AndItsNotYou".... doesn't work? :O
>
> Best regards,
> Timo Kreuzer
>
> Am 10.05.2016 um 17:03 schrieb [e-mail]:
> > Author: tfaber
> > Date: Tue May 10 15:03:56 2016
> > New Revision: 71307
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=71307&view=rev
> > Log:
> > [HAL]
> > Create a wrapper version of HalpEndSoftwareInterrupt that frees its
> >
> > stack before calling the noreturn version of the next software interrupt
> > handler. Fixes excessive stack usage when DPCs are queued in quick
> > succession.
> > CORE-11123 #resolve
> >
> > Added:
> >      trunk/reactos/hal/halx86/up/pic.S   (with props)
> > Modified:
> >      trunk/reactos/hal/halx86/pic.cmake
> >      trunk/reactos/hal/halx86/up/pic.c
> >
> > Modified: trunk/reactos/hal/halx86/pic.cmake
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/pic.cmake?rev=71...
> >
> > ==============================================================================
> > --- trunk/reactos/hal/halx86/pic.cmake  [iso-8859-1] (original)
> > +++ trunk/reactos/hal/halx86/pic.cmake  [iso-8859-1] Tue May 10 15:03:56
> > 2016
> > @@ -1,7 +1,8 @@
> >     list(APPEND HAL_PIC_ASM_SOURCE
> >       generic/systimer.S
> > generic/trap.S)
> > generic/trap.S
> > up/pic.S)
> > list(APPEND HAL_PIC_SOURCE
> >  generic/profil.c
> >
> > Added: trunk/reactos/hal/halx86/up/pic.S
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/up/pic.S?rev=713...
> >
> > ==============================================================================
> > --- trunk/reactos/hal/halx86/up/pic.S   (added)
> > +++ trunk/reactos/hal/halx86/up/pic.S   [iso-8859-1] Tue May 10 15:03:56
> > 2016
> > @@ -0,0 +1,48 @@
> > +/*
> > FILE:            hal/halx86/up/pic.S
> > COPYRIGHT:       See COPYING in the top level directory
> > PURPOSE:         HAL PIC Management and Control Code
> > PROGRAMMER:      Thomas Faber ([e-mail])
> > */
> >
> > +/* INCLUDES
> > ******************************************************************/
> >
> > +#include <asm.inc>
> >
> > +#include <ks386.inc>
> >
> > +EXTERN _HalpEndSoftwareInterrupt2@8:PROC
> >
> > +/* GLOBALS
> > *******************************************************************/
> >
> > +.data
> > +ASSUME CS:NOTHING, DS:NOTHING, ES:NOTHING, FS:NOTHING, GS:NOTHING
> >
> > +/* FUNCTIONS
> > *****************************************************************/
> >
> > +.code
> > +PUBLIC _HalpEndSoftwareInterrupt@8
> > +.PROC _HalpEndSoftwareInterrupt@8
> > FPO 0, 2, 0, 0, 0, FRAME_FPO
> > /* Call the C function with the same arguments we got */
> > push [esp+8]
> > push [esp+8]
> > call _HalpEndSoftwareInterrupt2@8
> > /* Check if we got a pointer back */
> > test eax, eax
> > jnz CallIntHandler
> > /* No? Just return */
> > ret 8
> >
> > +CallIntHandler:
> > /* We got a pointer to call. Since it won't return, free up our stack
> >   space, or we could end up with some nasty deep recursion */
> > mov ecx, [esp+8]
> > add esp, 12
> > jmp eax
> >
> > +.ENDP
> >
> > +END
> >
> > Propchange: trunk/reactos/hal/halx86/up/pic.S
> >  svn:eol-style = native
> >
> > Modified: trunk/reactos/hal/halx86/up/pic.c
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/up/pic.c?rev=713...
> >
> > ==============================================================================
> > --- trunk/reactos/hal/halx86/up/pic.c   [iso-8859-1] (original)
> > +++ trunk/reactos/hal/halx86/up/pic.c   [iso-8859-1] Tue May 10 15:03:56
> > 2016
> > @@ -11,6 +11,11 @@
> >   #include <hal.h>
> >   #define NDEBUG
> >   #include <debug.h>
> >
> > +VOID
> > +NTAPI
> > +HalpEndSoftwareInterrupt(IN KIRQL OldIrql,
> >                     IN PKTRAP_FRAME TrapFrame);
> > /* GLOBALS
> >
> > ********************************************************************/
> >   @@ -263,7 +268,7 @@
> >        * so it will always preempt until we reach PROFILE_LEVEL.
> >        */
> >       0b00000000000000000001011111110000, /* IRQL 20 */
> > 0b00000000000000000001001111110000, /* IRQL 20 */
> > 0b00000000000000000001001111110000, /* IRQL 21 */
> >  0b00000000000000000001000111110000, /* IRQL 22 */
> >  0b00000000000000000001000011110000, /* IRQL 23 */
> >  0b00000000000000000001000001110000, /* IRQL 24 */
> >
> > @@ -732,15 +737,17 @@
> >       KeGetPcr()->IRR &= ~(1 << Irql);
> >   }
> >   -VOID
> > -NTAPI
> > -HalpEndSoftwareInterrupt(IN KIRQL OldIrql,
> >                     IN PKTRAP_FRAME TrapFrame)
> >
> > +PHAL_SW_INTERRUPT_HANDLER_2ND_ENTRY
> > +NTAPI
> > +HalpEndSoftwareInterrupt2(IN KIRQL OldIrql,
> >                      IN PKTRAP_FRAME TrapFrame)
> > {
> >     ULONG PendingIrql, PendingIrqlMask, PendingIrqMask;
> >     PKPCR Pcr = KeGetPcr();
> >     PIC_MASK Mask;
> > UNREFERENCED_PARAMETER(TrapFrame);
> > /* Set old IRQL */
> > Pcr->Irql = OldIrql;
> > @@ -749,10 +756,10 @@
> >     {
> >         /* Check for pending software interrupts and compare with
> >
> > current IRQL */
> >           PendingIrqlMask = Pcr->IRR & FindHigherIrqlMask[OldIrql];
> >    if (!PendingIrqlMask) return;
> >    if (!PendingIrqlMask) return NULL;
> >       /* Check for in-service delayed interrupt */
> >    if (Pcr->IrrActive & 0xFFFFFFF0) return;
> >    if (Pcr->IrrActive & 0xFFFFFFF0) return NULL;
> >       /* Check if pending IRQL affects hardware state */
> >     BitScanReverse(&PendingIrql, PendingIrqlMask);
> >
> > @@ -777,10 +784,11 @@
> >           else
> >           {
> >               /* No need to loop checking for hardware interrupts */
> >        SWInterruptHandlerTable2[PendingIrql](TrapFrame);
> >        UNREACHABLE;
> >        return SWInterruptHandlerTable2[PendingIrql];
> >     }
> > }
> > return NULL;
> > }
> > /* EDGE INTERRUPT DISMISSAL FUNCTIONS
> >
> > *****************************************/
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev