Looks like we've encountered an issue with this
KiTrapReturnNoSegmentsRet8 approach. As seen in
https://jira.reactos.org/browse/CORE-11115, when a large number of
interrupts happen, this will cause us to run out of kernel stack.

Since the popfd / ret 8 steps do not happen atomically, interrupts will
be re-enabled before those 12 bytes of stack are freed, thus many of
them happening in a row will cause us to run out.

Thoughts?

Thanks!
-Thomas



On 2013-10-19 13:33, [email protected] wrote:
> Author: tkreuzer
> Date: Sat Oct 19 11:33:34 2013
> New Revision: 60701
> 
> URL: http://svn.reactos.org/svn/reactos?rev=60701&view=rev
> Log:
> [NTOSKRNL]
> - Introduce a new and faster way to return to kernel mode from traps by using 
> a ret 8 instruction instead of an iret.
> - Make use of KiUserTrap where appropriate
> - Remove some pointless toplevel volatile
> 
> Modified:
>     trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
>     trunk/reactos/ntoskrnl/include/internal/i386/ke.h
>     trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h
>     trunk/reactos/ntoskrnl/ke/i386/cpu.c
>     trunk/reactos/ntoskrnl/ke/i386/exp.c
>     trunk/reactos/ntoskrnl/ke/i386/patpge.c
>     trunk/reactos/ntoskrnl/ke/i386/trap.s
>     trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
>     trunk/reactos/ntoskrnl/ke/i386/usercall.c
>     trunk/reactos/ntoskrnl/ke/time.c
> 
> Modified: trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S   [iso-8859-1] 
> (original)
> +++ trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S   [iso-8859-1] 
> Sat Oct 19 11:33:34 2013
> @@ -241,6 +241,7 @@
>  #define KI_EXIT_RET           HEX(080)
>  #define KI_EXIT_IRET          HEX(100)
>  #define KI_EDITED_FRAME       HEX(200)
> +#define KI_EXIT_RET8          HEX(400)
>  #define KI_RESTORE_VOLATILES  (KI_RESTORE_EAX OR KI_RESTORE_ECX_EDX)
>  
>  MACRO(KiTrapExitStub, Name, Flags)
> @@ -248,15 +249,15 @@
>  PUBLIC @&Name&@4
>  @&Name&@4:
>  
> -    if (Flags AND KI_RESTORE_EFLAGS)
> +    if (Flags AND KI_EXIT_RET8) OR (Flags AND KI_EXIT_IRET)
> +
> +        /* This is the IRET frame */
> +        OffsetEsp = KTRAP_FRAME_EIP
> +
> +    elseif (Flags AND KI_RESTORE_EFLAGS)
>  
>          /* We will pop EFlags off the stack */
>          OffsetEsp = KTRAP_FRAME_EFLAGS
> -
> -    elseif (Flags AND KI_EXIT_IRET)
> -
> -        /* This is the IRET frame */
> -        OffsetEsp = KTRAP_FRAME_EIP
>  
>      else
>  
> @@ -335,6 +336,13 @@
>  
>      if (Flags AND KI_RESTORE_EFLAGS)
>  
> +        if (Flags AND KI_EXIT_RET8)
> +
> +            /* We are at the IRET frame, so push EFLAGS first */
> +            push dword ptr [esp + 8]
> +
> +        endif
> +
>          /* Restore EFLAGS */
>          popfd
>  
> @@ -357,6 +365,11 @@
>          /* Return to kernel mode with a jmp */
>          jmp edx
>  
> +    elseif (Flags AND KI_EXIT_RET8)
> +
> +        /* Return to kernel mode with a ret 8 */
> +        ret 8
> +
>      elseif (Flags AND KI_EXIT_RET)
>  
>          /* Return to kernel mode with a ret */
> 
> Modified: trunk/reactos/ntoskrnl/include/internal/i386/ke.h
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/ke.h?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/include/internal/i386/ke.h [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/include/internal/i386/ke.h [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -383,13 +383,13 @@
>  ULONG_PTR
>  NTAPI
>  Ki386EnableGlobalPage(
> -    IN volatile ULONG_PTR Context
> +    IN ULONG_PTR Context
>  );
>  
>  ULONG_PTR
>  NTAPI
>  Ki386EnableTargetLargePage(
> -    IN volatile ULONG_PTR Context
> +    IN ULONG_PTR Context
>  );
>  
>  BOOLEAN
> 
> Modified: trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h     [iso-8859-1] 
> (original)
> +++ trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h     [iso-8859-1] 
> Sat Oct 19 11:33:34 2013
> @@ -163,7 +163,7 @@
>      }
>  
>      /* Check DR values */
> -    if (TrapFrame->SegCs & MODE_MASK)
> +    if (KiUserTrap(TrapFrame)
>      {
>          /* Check for active debugging */
>          if (KeGetCurrentThread()->Header.DebugActive)
> @@ -244,6 +244,7 @@
>  DECLSPEC_NORETURN VOID FASTCALL KiEditedTrapReturn(IN PKTRAP_FRAME 
> TrapFrame);
>  DECLSPEC_NORETURN VOID FASTCALL KiTrapReturn(IN PKTRAP_FRAME TrapFrame);
>  DECLSPEC_NORETURN VOID FASTCALL KiTrapReturnNoSegments(IN PKTRAP_FRAME 
> TrapFrame);
> +DECLSPEC_NORETURN VOID FASTCALL KiTrapReturnNoSegmentsRet8(IN PKTRAP_FRAME 
> TrapFrame);
>  
>  typedef
>  ATTRIB_NORETURN
> @@ -382,7 +383,7 @@
>      TrapFrame->Dr7 = 0;
>  
>      /* Check if the frame was from user mode or v86 mode */
> -    if ((TrapFrame->SegCs & MODE_MASK) ||
> +    if (KiUserTrap(TrapFrame) ||
>          (TrapFrame->EFlags & EFLAGS_V86_MASK))
>      {
>          /* Check for active debugging */
> @@ -411,7 +412,7 @@
>      TrapFrame->Dr7 = 0;
>  
>      /* Check if the frame was from user mode or v86 mode */
> -    if ((TrapFrame->SegCs & MODE_MASK) ||
> +    if (KiUserTrap(TrapFrame) ||
>          (TrapFrame->EFlags & EFLAGS_V86_MASK))
>      {
>          /* Check for active debugging */
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/cpu.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/cpu.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/cpu.c      [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/cpu.c      [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -1535,7 +1535,7 @@
>      if (TargetAffinity)
>      {
>          /* Sanity check */
> -        ASSERT(Prcb == (volatile PKPRCB)KeGetCurrentPrcb());
> +        ASSERT(Prcb == KeGetCurrentPrcb());
>  
>          /* FIXME: TODO */
>          ASSERTMSG("Not yet implemented\n", FALSE);
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/exp.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/exp.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/exp.c      [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/exp.c      [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -137,7 +137,7 @@
>  KiEspFromTrapFrame(IN PKTRAP_FRAME TrapFrame)
>  {
>      /* Check if this is user-mode or V86 */
> -    if ((TrapFrame->SegCs & MODE_MASK) ||
> +    if (KiUserTrap(TrapFrame) ||
>          (TrapFrame->EFlags & EFLAGS_V86_MASK))
>      {
>          /* Return it directly */
> @@ -175,7 +175,7 @@
>      Previous = KiEspFromTrapFrame(TrapFrame);
>  
>      /* Check if this is user-mode or V86 */
> -    if ((TrapFrame->SegCs & MODE_MASK) ||
> +    if (KiUserTrap(TrapFrame) ||
>          (TrapFrame->EFlags & EFLAGS_V86_MASK))
>      {
>          /* Write it directly */
> @@ -225,7 +225,7 @@
>          /* Just return it */
>          return TrapFrame->HardwareSegSs;
>      }
> -    else if (TrapFrame->SegCs & MODE_MASK)
> +    else if (KiUserTrap(TrapFrame))
>      {
>          /* User mode, return the User SS */
>          return TrapFrame->HardwareSegSs | RPL_MASK;
> @@ -251,7 +251,7 @@
>          /* Just write it */
>          TrapFrame->HardwareSegSs = Ss;
>      }
> -    else if (TrapFrame->SegCs & MODE_MASK)
> +    else if (KiUserTrap(TrapFrame))
>      {
>          /* Usermode, save the User SS */
>          TrapFrame->HardwareSegSs = Ss | RPL_MASK;
> @@ -398,7 +398,7 @@
>              TrapFrame->V86Fs = Context->SegFs;
>              TrapFrame->V86Gs = Context->SegGs;
>          }
> -        else if (!(TrapFrame->SegCs & MODE_MASK))
> +        else if (!KiUserTrap(TrapFrame))
>          {
>              /* For kernel mode, write the standard values */
>              TrapFrame->SegDs = KGDT_R3_DATA | RPL_MASK;
> @@ -429,7 +429,7 @@
>  
>      /* Handle the extended registers */
>      if (((ContextFlags & CONTEXT_EXTENDED_REGISTERS) ==
> -        CONTEXT_EXTENDED_REGISTERS) && (TrapFrame->SegCs & MODE_MASK))
> +        CONTEXT_EXTENDED_REGISTERS) && KiUserTrap(TrapFrame))
>      {
>          /* Get the FX Area */
>          FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1);
> @@ -463,7 +463,7 @@
>  
>      /* Handle the floating point state */
>      if (((ContextFlags & CONTEXT_FLOATING_POINT) ==
> -        CONTEXT_FLOATING_POINT) && (TrapFrame->SegCs & MODE_MASK))
> +        CONTEXT_FLOATING_POINT) && KiUserTrap(TrapFrame))
>      {
>          /* Get the FX Area */
>          FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1);
> @@ -692,7 +692,7 @@
>  
>      /* Handle extended registers */
>      if (((Context->ContextFlags & CONTEXT_EXTENDED_REGISTERS) ==
> -        CONTEXT_EXTENDED_REGISTERS) && (TrapFrame->SegCs & MODE_MASK))
> +        CONTEXT_EXTENDED_REGISTERS) && KiUserTrap(TrapFrame))
>      {
>          /* Get the FX Save Area */
>          FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1);
> @@ -712,7 +712,7 @@
>  
>      /* Handle Floating Point */
>      if (((Context->ContextFlags & CONTEXT_FLOATING_POINT) ==
> -        CONTEXT_FLOATING_POINT) && (TrapFrame->SegCs & MODE_MASK))
> +        CONTEXT_FLOATING_POINT) && KiUserTrap(TrapFrame))
>      {
>          /* Get the FX Save Area */
>          FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1);
> @@ -1045,6 +1045,13 @@
>                  }
>              }
>              _SEH2_END;
> +
> +            DPRINT("First chance exception in %.16s, ExceptionCode: %lx, 
> ExceptionAddress: %p, P0: %lx, P1: %lx\n",
> +                   PsGetCurrentProcess()->ImageFileName,
> +                   ExceptionRecord->ExceptionCode,
> +                   ExceptionRecord->ExceptionAddress,
> +                   ExceptionRecord->ExceptionInformation[0],
> +                   ExceptionRecord->ExceptionInformation[1]);
>          }
>  
>          /* Try second chance */
> @@ -1060,11 +1067,13 @@
>          }
>  
>          /* 3rd strike, kill the process */
> -        DPRINT1("Kill %.16s, ExceptionCode: %lx, ExceptionAddress: %p, 
> BaseAddress: %p\n",
> +        DPRINT1("Kill %.16s, ExceptionCode: %lx, ExceptionAddress: %p, 
> BaseAddress: %p, P0: %lx, P1: %lx\n",
>                  PsGetCurrentProcess()->ImageFileName,
>                  ExceptionRecord->ExceptionCode,
>                  ExceptionRecord->ExceptionAddress,
> -                PsGetCurrentProcess()->SectionBaseAddress);
> +                PsGetCurrentProcess()->SectionBaseAddress,
> +                ExceptionRecord->ExceptionInformation[0],
> +                ExceptionRecord->ExceptionInformation[1]);
>  
>          ZwTerminateProcess(NtCurrentProcess(), 
> ExceptionRecord->ExceptionCode);
>          KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED,
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/patpge.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/patpge.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/patpge.c   [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/patpge.c   [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -20,9 +20,9 @@
>  ULONG_PTR
>  NTAPI
>  INIT_FUNCTION
> -Ki386EnableGlobalPage(IN volatile ULONG_PTR Context)
> -{
> -    volatile PLONG Count = (PLONG)Context;
> +Ki386EnableGlobalPage(IN ULONG_PTR Context)
> +{
> +    PLONG Count = (PLONG)Context;
>      ULONG Cr4, Cr3;
>  
>      /* Disable interrupts */
> @@ -144,7 +144,7 @@
>          if (PageTable)
>              *PageTable = (PHARDWARE_PTE)(Pde->PageFrameNumber << PAGE_SHIFT);
>      }
> -    
> +
>      return TRUE;
>  }
>  
> @@ -172,7 +172,7 @@
>      /* Get PTE of VirtualPtr, make it valid, and map PhysicalPtr there */
>      Pte = &PageTable[(VirtualPtr >> 12) & ((1 << PTE_BITS) - 1)];
>      Pte->Valid = 1;
> -    Pte->PageFrameNumber = PhysicalPtr.QuadPart >> PAGE_SHIFT;
> +    Pte->PageFrameNumber = (PFN_NUMBER)(PhysicalPtr.QuadPart >> PAGE_SHIFT);
>  
>      return TRUE;
>  }
> @@ -189,7 +189,7 @@
>      PhysicalPtr = MmGetPhysicalAddress(VirtualPtr);
>  
>      /* Map its physical address in the page table provided by the caller */
> -    Pte->PageFrameNumber = PhysicalPtr.QuadPart >> PAGE_SHIFT;
> +    Pte->PageFrameNumber = (PFN_NUMBER)(PhysicalPtr.QuadPart >> PAGE_SHIFT);
>  }
>  
>  BOOLEAN
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/trap.s     [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/trap.s     [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -166,6 +166,7 @@
>  KiTrapExitStub KiEditedTrapReturn,        (KI_RESTORE_VOLATILES OR 
> KI_RESTORE_EFLAGS OR KI_EDITED_FRAME OR KI_EXIT_RET)
>  KiTrapExitStub KiTrapReturn,              (KI_RESTORE_VOLATILES OR 
> KI_RESTORE_SEGMENTS OR KI_EXIT_IRET)
>  KiTrapExitStub KiTrapReturnNoSegments,    (KI_RESTORE_VOLATILES OR 
> KI_EXIT_IRET)
> +KiTrapExitStub KiTrapReturnNoSegmentsRet8,(KI_RESTORE_VOLATILES OR 
> KI_RESTORE_EFLAGS OR KI_EXIT_RET8)
>  
>  #ifdef _MSC_VER
>  EXTERN _PsConvertToGuiThread@0:PROC
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -100,7 +100,7 @@
>      if (__builtin_expect(TrapFrame->Dr7 & ~DR7_RESERVED_MASK, 0))
>      {
>          /* Check if the frame was from user mode or v86 mode */
> -        if ((TrapFrame->SegCs & MODE_MASK) ||
> +        if (KiUserTrap(TrapFrame) ||
>              (TrapFrame->EFlags & EFLAGS_V86_MASK))
>          {
>              /* Handle debug registers */
> @@ -124,13 +124,13 @@
>      if (TrapFrame->EFlags & EFLAGS_V86_MASK) 
> KiTrapReturnNoSegments(TrapFrame);
>  
>      /* Check for user mode exit */
> -    if (TrapFrame->SegCs & MODE_MASK) KiTrapReturn(TrapFrame);
> +    if (KiUserTrap(TrapFrame)) KiTrapReturn(TrapFrame);
>  
>      /* Check for edited frame */
>      if (KiIsFrameEdited(TrapFrame)) KiEditedTrapReturn(TrapFrame);
>  
>      /* Exit the trap to kernel mode */
> -    KiTrapReturnNoSegments(TrapFrame);
> +    KiTrapReturnNoSegmentsRet8(TrapFrame);
>  }
>  
>  DECLSPEC_NORETURN
> @@ -152,7 +152,7 @@
>      KeGetCurrentThread()->PreviousMode = 
> (CCHAR)TrapFrame->PreviousPreviousMode;
>  
>      /* Check for user mode exit */
> -    if (TrapFrame->SegCs & MODE_MASK)
> +    if (KiUserTrap(TrapFrame))
>      {
>          /* Check if we were single stepping */
>          if (TrapFrame->EFlags & EFLAGS_TF)
> @@ -186,13 +186,13 @@
>      if (TrapFrame->EFlags & EFLAGS_V86_MASK) 
> KiTrapReturnNoSegments(TrapFrame);
>  
>      /* Check for user mode exit */
> -    if (TrapFrame->SegCs & MODE_MASK) KiTrapReturn(TrapFrame);
> +    if (KiUserTrap(TrapFrame)) KiTrapReturn(TrapFrame);
>  
>      /* Check for edited frame */
>      if (KiIsFrameEdited(TrapFrame)) KiEditedTrapReturn(TrapFrame);
>  
>      /* Exit the trap to kernel mode */
> -    KiTrapReturnNoSegments(TrapFrame);
> +    KiTrapReturnNoSegmentsRet8(TrapFrame);
>  }
>  
>  
> @@ -1250,10 +1250,10 @@
>      /* Call the access fault handler */
>      Status = MmAccessFault(TrapFrame->ErrCode & 1,
>                             (PVOID)Cr2,
> -                           TrapFrame->SegCs & MODE_MASK,
> +                           KiUserTrap(TrapFrame),
>                             TrapFrame);
>      if (NT_SUCCESS(Status)) KiEoiHelper(TrapFrame);
> -    
> +
>      /* Check for syscall fault */
>  #if 0
>      if ((TrapFrame->Eip == (ULONG_PTR)CopyParams) ||
> @@ -1541,7 +1541,7 @@
>      TrapFrame->Dr7 = 0;
>  
>      /* Check if the frame was from user mode */
> -    if (TrapFrame->SegCs & MODE_MASK)
> +    if (KiUserTrap(TrapFrame))
>      {
>          /* Check for active debugging */
>          if (KeGetCurrentThread()->Header.DebugActive & 0xFF)
> 
> Modified: trunk/reactos/ntoskrnl/ke/i386/usercall.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/usercall.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/usercall.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/usercall.c [iso-8859-1] Sat Oct 19 
> 11:33:34 2013
> @@ -67,7 +67,7 @@
>      _SEH2_TRY
>      {
>          /* Sanity check */
> -        ASSERT((TrapFrame->SegCs & MODE_MASK) != KernelMode);
> +        ASSERT(KiUserTrap(TrapFrame));
>  
>          /* Get the aligned size */
>          AlignedEsp = Context.Esp & ~3;
> @@ -210,7 +210,7 @@
>  }
>  
>  
> -/* 
> +/*
>   * Stack layout for KiUserModeCallout:
>   * ----------------------------------
>   * KCALLOUT_FRAME.ResultLength    <= 2nd Parameter to KiCallUserMode
> 
> Modified: trunk/reactos/ntoskrnl/ke/time.c
> URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/time.c?rev=60701&r1=60700&r2=60701&view=diff
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/time.c  [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/time.c  [iso-8859-1] Sat Oct 19 11:33:34 2013
> @@ -153,7 +153,7 @@
>  
>      /* Check if we came from user mode */
>  #ifndef _M_ARM
> -    if ((TrapFrame->SegCs & MODE_MASK) || (TrapFrame->EFlags & 
> EFLAGS_V86_MASK))
> +    if (KiUserTrap(TrapFrame) || (TrapFrame->EFlags & EFLAGS_V86_MASK))
>  #else
>      if (TrapFrame->PreviousMode == UserMode)
>  #endif
> 
> 


_______________________________________________
Ros-dev mailing list
[email protected]
http://www.reactos.org/mailman/listinfo/ros-dev

Reply via email to