Re: [ros-dev] [ros-diffs] [tkreuzer] 60701: [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 - Remov... - Ros-dev

14 Apr 2016

Wait, are you suggesting that "doing things better than Microsoft" and
"those idiots have no idea what they're doing" and "we shouldn't just
be copying the instructions they use".... doesn't work? :O
Best regards,
Alex Ionescu
On Thu, Apr 14, 2016 at 8:09 AM, Thomas Faber thomas.faber@reactos.org wrote:
...
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, tkreuzer@svn.reactos.org 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/i...
==============================================================================

--- 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/i...
==============================================================================
--- 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/i...
==============================================================================
--- 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=...
==============================================================================
--- 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=...
==============================================================================
--- 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?r...
==============================================================================
--- 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...
==============================================================================
--- 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...
==============================================================================
--- 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...
==============================================================================
--- 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=6070...
==============================================================================
--- 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
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

    