- Rewrite usermode callbacks. These changes should greatly optimize graphic operations. After these changes, my "idle" CPU Usage in taskmgr went from 7-8% to 4-5%, while on the performace page, from 15-18% to 10-13%:

* Do not use ugly and messy code to create linked stacks and other such resource-wasting steps. Use our newly implemented 60KB stack support and MmGrowKernelStack when needed.
  * Write all the low-level code in assembly instead of relying on structures and hodgepodge code.
  * Add debugging/detection features for invalid calls, such as invalid IRQL, APCs being disabled, invalid previous mode detection (this allowed me to fix the KWAIT_BLOCK bug today).
  * Finally fix the last (I hope) remaning trap frame bug issue related to V86 mode bias. One of the "hacks" in syscall.S has already been removed and I can now do the promised cleanup.
  * Allow some failulre cases in callbacks (not all implemented) and extend stack space for future use of SEH in the ntdll dispatcher.
  * Fix win32k to use callbacks properly: the system fills out *Result and *ResultLength, not the caller.
  * Use SEH (ProbeForWrite) in callbacks to detect invalid user-mode memory.
  * Save NPX State and ExceptionList across callbacks (I think this wasn't fully properly done in all cases).


svn path=/trunk/; revision=20794

reactos/ntoskrnl/ke/i386/syscall.S

@@ -470,23 +470,14 @@ V86_Exit_Return:
     ja RestoreAll
 // ==================== END IF FULL RESTORE NEEDED ====================//
 
- /* Skip debug information and unsaved registers */
- //badbadbad
-    add esp, 0x30
-    pop gs
-    pop es
-    pop ds
-    add esp, 0x14
-//badbadbad
-
     /* Restore FS */
 RestoreFs:
-    //lea esp, [ebp+KTRAP_FRAME_FS] <= BUG IN WIN32K CALLBACKS! STACK GETS SMASHED
+    lea esp, [ebp+KTRAP_FRAME_FS]
     pop fs
 
 CommonStackClean:
     /* Skip debug information and unsaved registers */
-    //lea esp, [ebp+KTRAP_FRAME_EDI] <= BUG IN WIN32K CALLBACKS! STACK GETS SMASHED
+    lea esp, [ebp+KTRAP_FRAME_EDI]
     pop edi
     pop esi
     pop ebx

reactos/ntoskrnl/ke/i386/usercall_asm.S

@@ -248,9 +248,9 @@ GrowFailed:
  * @remark This call MUST be paired with KeUserModeCallback.
  *
  *--*/
-.globl _NtCallbackReturn2@12
+.globl _NtCallbackReturn@12
-.func NtCallbackReturn2@12
+.func NtCallbackReturn@12
-_NtCallbackReturn2@12:
+_NtCallbackReturn@12:
 
     /* Get the current thread and make sure we have a callback stack */
     mov eax, fs:[KPCR_CURRENT_THREAD]

reactos/ntoskrnl/ke/usercall.c

@@ -3,7 +3,6 @@
  * PROJECT:         ReactOS kernel
  * FILE:            ntoskrnl/ke/usercall.c
  * PURPOSE:         User-Mode callbacks. Portable part.
- *
  * PROGRAMMERS:     Alex Ionescu (alex@relsoft.net)
  */
 
@@ -13,148 +12,20 @@
 #define NDEBUG
 #include <internal/debug.h>
 
-#if defined (ALLOC_PRAGMA)
-#pragma alloc_text(INIT, PsInitialiseW32Call)
-#endif
-
-/* FUNCTIONS *****************************************************************/
-
-#if ALEX_CB_REWRITE
-
 NTSTATUS
 STDCALL
-KiSwitchToUserMode(IN PVOID *OutputBuffer,
+KiCallUserMode(
-                   IN PULONG OutputLength);
+    IN PVOID *OutputBuffer,
+    IN PULONG OutputLength
+);
 
-#else
+PULONG
+STDCALL
+KiGetUserModeStackAddress(
+    VOID
+);
 
-typedef struct _NTW32CALL_SAVED_STATE
+/* FUNCTIONS *****************************************************************/
-{
-  ULONG_PTR SavedStackLimit;
-  PVOID SavedStackBase;
-  PVOID SavedInitialStack;
-  PVOID CallerResult;
-  PULONG CallerResultLength;
-  PNTSTATUS CallbackStatus;
-  PKTRAP_FRAME SavedTrapFrame;
-  PVOID SavedCallbackStack;
-  PVOID SavedExceptionStack;
-} NTW32CALL_SAVED_STATE, *PNTW32CALL_SAVED_STATE;
-
-typedef struct
-{
-  PVOID BaseAddress;
-  LIST_ENTRY ListEntry;
-} NTW32CALL_CALLBACK_STACK, *PNTW32CALL_CALLBACK_STACK;
-
-KSPIN_LOCK CallbackStackListLock;
-static LIST_ENTRY CallbackStackListHead;
-
-VOID 
-INIT_FUNCTION
-NTAPI
-PsInitialiseW32Call(VOID)
-{
-  InitializeListHead(&CallbackStackListHead);
-  KeInitializeSpinLock(&CallbackStackListLock);
-}
-
-VOID STATIC
-PsFreeCallbackStackPage(PVOID Context, MEMORY_AREA* MemoryArea, PVOID Address,
-			PFN_TYPE Page, SWAPENTRY SwapEntry,
-			BOOLEAN Dirty)
-{
-  ASSERT(SwapEntry == 0);
-  if (Page != 0)
-    {
-      MmReleasePageMemoryConsumer(MC_NPPOOL, Page);
-    }
-}
-
-VOID STATIC
-PsFreeCallbackStack(PVOID StackLimit)
-{
-  MmLockAddressSpace(MmGetKernelAddressSpace());
-  MmFreeMemoryAreaByPtr(MmGetKernelAddressSpace(),
-		        StackLimit,
-		        PsFreeCallbackStackPage,
-		        NULL);
-  MmUnlockAddressSpace(MmGetKernelAddressSpace());
-}
-
-VOID
-PsFreeCallbackStacks(VOID)
-{
-  PLIST_ENTRY CurrentListEntry;
-  PNTW32CALL_CALLBACK_STACK Current;
-
-  while (!IsListEmpty(&CallbackStackListHead))
-    {
-      CurrentListEntry = RemoveHeadList(&CallbackStackListHead);
-      Current = CONTAINING_RECORD(CurrentListEntry, NTW32CALL_CALLBACK_STACK,
-				  ListEntry);
-      PsFreeCallbackStack(Current->BaseAddress);
-      ExFreePool(Current);
-    }
-}
-
-PVOID STATIC
-PsAllocateCallbackStack(ULONG StackSize)
-{
-  PVOID KernelStack = NULL;
-  NTSTATUS Status;
-  PMEMORY_AREA StackArea;
-  ULONG i, j;
-  PHYSICAL_ADDRESS BoundaryAddressMultiple;
-  PPFN_TYPE Pages = alloca(sizeof(PFN_TYPE) * (StackSize /PAGE_SIZE));
-
-
-  BoundaryAddressMultiple.QuadPart = 0;
-  StackSize = PAGE_ROUND_UP(StackSize);
-  MmLockAddressSpace(MmGetKernelAddressSpace());
-  Status = MmCreateMemoryArea(MmGetKernelAddressSpace(),
-			      MEMORY_AREA_KERNEL_STACK,
-			      &KernelStack,
-			      StackSize,
-			      PAGE_READWRITE,
-			      &StackArea,
-			      FALSE,
-			      0,
-			      BoundaryAddressMultiple);
-  MmUnlockAddressSpace(MmGetKernelAddressSpace());
-  if (!NT_SUCCESS(Status))
-    {
-      DPRINT("Failed to create thread stack\n");
-      return(NULL);
-    }
-  for (i = 0; i < (StackSize / PAGE_SIZE); i++)
-    {
-      Status = MmRequestPageMemoryConsumer(MC_NPPOOL, TRUE, &Pages[i]);
-      if (!NT_SUCCESS(Status))
-	{
-	  for (j = 0; j < i; j++)
-	  {
-	    MmReleasePageMemoryConsumer(MC_NPPOOL, Pages[j]);
-	  }
-	  return(NULL);
-	}
-    }
-  Status = MmCreateVirtualMapping(NULL,
-				  KernelStack,
-				  PAGE_READWRITE,
-				  Pages,
-				  StackSize / PAGE_SIZE);
-  if (!NT_SUCCESS(Status))
-    {
-      for (i = 0; i < (StackSize / PAGE_SIZE); i++)
-        {
-	  MmReleasePageMemoryConsumer(MC_NPPOOL, Pages[i]);
-	}
-      return(NULL);
-    }
-  return(KernelStack);
-}
-#endif
 
 /*
  * @implemented
@@ -167,92 +38,63 @@ KeUserModeCallback(IN ULONG RoutineIndex,
                    OUT PVOID *Result,
                    OUT PULONG ResultLength)
 {
-  PETHREAD Thread;
+    ULONG_PTR NewStack, OldStack;
-  PVOID NewStack;
+    PULONG UserEsp;
-  ULONG_PTR StackSize;
+    NTSTATUS CallbackStatus;
-  PKTRAP_FRAME NewFrame;
+    PEXCEPTION_REGISTRATION_RECORD ExceptionList;
-  PULONG UserEsp;
+    DPRINT("KeUserModeCallback(RoutineIndex %d, Argument %X, ArgumentLength %d)\n",
-  KIRQL oldIrql;
+            RoutineIndex, Argument, ArgumentLength);
-  NTSTATUS CallbackStatus;
+    ASSERT(KeGetCurrentThread()->ApcState.KernelApcInProgress == FALSE);
-  NTW32CALL_SAVED_STATE SavedState;
+    ASSERT(KeGetPreviousMode() == UserMode);
-  PNTW32CALL_CALLBACK_STACK AssignedStack;
 
-  PAGED_CODE();
+    /* Get the current user-mode stack */
+    UserEsp = KiGetUserModeStackAddress();
+    OldStack = *UserEsp;
 
-  DPRINT("KeUserModeCallback(RoutineIndex %d, Argument %X, ArgumentLength %d)\n",
+    /* Enter a SEH Block */
-	  RoutineIndex, Argument, ArgumentLength);
+    _SEH_TRY
-
-  Thread = PsGetCurrentThread();
-
-  /* Set up the new kernel and user environment. */
-  StackSize = (ULONG_PTR)Thread->Tcb.StackBase - Thread->Tcb.StackLimit;
-  KeAcquireSpinLock(&CallbackStackListLock, &oldIrql);
-  if (IsListEmpty(&CallbackStackListHead))
     {
-      KeReleaseSpinLock(&CallbackStackListLock, oldIrql);
+        /* Calculate and align the stack size */
-      NewStack = PsAllocateCallbackStack(StackSize);
+        NewStack = (OldStack - ArgumentLength) & ~3;
-      AssignedStack = ExAllocatePool(NonPagedPool,
+
-				     sizeof(NTW32CALL_CALLBACK_STACK));
+        /* Make sure it's writable */
-      AssignedStack->BaseAddress = NewStack;
+        ProbeForWrite((PVOID)(NewStack - 6 * sizeof(ULONG_PTR)),
+                      ArgumentLength + 6 * sizeof(ULONG_PTR),
+                      sizeof(CHAR));
+
+        /* Copy the buffer into the stack */
+        RtlCopyMemory((PVOID)NewStack, Argument, ArgumentLength);
+
+        /* Write the arguments */
+        NewStack -= 24;
+        *(PULONG)NewStack = 0;
+        *(PULONG)(NewStack + 4) = RoutineIndex;
+        *(PULONG)(NewStack + 8) = (NewStack + 24);
+        *(PULONG)(NewStack + 12) = ArgumentLength;
+
+        /* Save the exception list */
+        ExceptionList = KeGetCurrentThread()->Teb->Tib.ExceptionList;
+
+        /* Jump to user mode */
+        *UserEsp = NewStack;
+        CallbackStatus = KiCallUserMode(Result, ResultLength);
+
+        /* FIXME: Handle user-mode exception status */
+
+        /* Restore exception list */
+        KeGetCurrentThread()->Teb->Tib.ExceptionList = ExceptionList;
     }
-  else
+    _SEH_HANDLE
     {
-      PLIST_ENTRY StackEntry;
+        CallbackStatus = _SEH_GetExceptionCode();
-
-      StackEntry = RemoveHeadList(&CallbackStackListHead);
-      KeReleaseSpinLock(&CallbackStackListLock, oldIrql);
-      AssignedStack = CONTAINING_RECORD(StackEntry, NTW32CALL_CALLBACK_STACK,
-					ListEntry);
-      NewStack = AssignedStack->BaseAddress;
-      RtlZeroMemory(NewStack, StackSize);
     }
-  /* FIXME: Need to check whether we were interrupted from v86 mode. */
+    _SEH_END;
-  RtlCopyMemory((char*)NewStack + StackSize - sizeof(KTRAP_FRAME) - sizeof(FX_SAVE_AREA),
-                Thread->Tcb.TrapFrame, sizeof(KTRAP_FRAME) - (4 * sizeof(ULONG)));
-  NewFrame = (PKTRAP_FRAME)((char*)NewStack + StackSize - sizeof(KTRAP_FRAME) - sizeof(FX_SAVE_AREA));
-  /* We need the stack pointer to remain 4-byte aligned */
-  NewFrame->HardwareEsp -= (((ArgumentLength + 3) & (~ 0x3)) + (4 * sizeof(ULONG)));
-  NewFrame->Eip = (ULONG)KeUserCallbackDispatcher;
-  UserEsp = (PULONG)NewFrame->HardwareEsp;
-  UserEsp[0] = 0;     /* Return address. */
-  UserEsp[1] = RoutineIndex;
-  UserEsp[2] = (ULONG)&UserEsp[4];
-  UserEsp[3] = ArgumentLength;
-  RtlCopyMemory((PVOID)&UserEsp[4], Argument, ArgumentLength);
 
-  /* Switch to the new environment and return to user-mode. */
+    /* FIXME: Flush GDI Batch */
-  KeRaiseIrql(HIGH_LEVEL, &oldIrql);
-  SavedState.SavedStackLimit = Thread->Tcb.StackLimit;
-  SavedState.SavedStackBase = Thread->Tcb.StackBase;
-  SavedState.SavedInitialStack = Thread->Tcb.InitialStack;
-  SavedState.CallerResult = Result;
-  SavedState.CallerResultLength = ResultLength;
-  SavedState.CallbackStatus = &CallbackStatus;
-  SavedState.SavedTrapFrame = Thread->Tcb.TrapFrame;
-  SavedState.SavedCallbackStack = Thread->Tcb.CallbackStack;
-  SavedState.SavedExceptionStack = (PVOID)KeGetCurrentKPCR()->TSS->Esp0;
-  if ((Thread->Tcb.NpxState & NPX_STATE_VALID) &&
-      &Thread->Tcb != KeGetCurrentPrcb()->NpxThread)
-    {
-      RtlCopyMemory((char*)NewStack + StackSize - sizeof(FX_SAVE_AREA),
-                    (char*)SavedState.SavedInitialStack - sizeof(FX_SAVE_AREA),
-                    sizeof(FX_SAVE_AREA));
-    }
-  Thread->Tcb.InitialStack = Thread->Tcb.StackBase = (char*)NewStack + StackSize;
-  Thread->Tcb.StackLimit = (ULONG)NewStack;
-  Thread->Tcb.KernelStack = (char*)NewStack + StackSize - sizeof(KTRAP_FRAME) - sizeof(FX_SAVE_AREA);
-  KeGetCurrentKPCR()->TSS->Esp0 = (ULONG)Thread->Tcb.InitialStack - sizeof(FX_SAVE_AREA) - 0x10;
-  KePushAndStackSwitchAndSysRet((ULONG)&SavedState, Thread->Tcb.KernelStack);
 
-  /*
+    /* Restore stack and return */
-   * The callback return will have already restored most of the state we
+    *UserEsp = OldStack;
-   * modified.
+    return CallbackStatus;
-   */
-  KeLowerIrql(DISPATCH_LEVEL);
-  KeAcquireSpinLockAtDpcLevel(&CallbackStackListLock);
-  InsertTailList(&CallbackStackListHead, &AssignedStack->ListEntry);
-  KeReleaseSpinLock(&CallbackStackListLock, PASSIVE_LEVEL);
-  return(CallbackStatus);
 }
 
 /* EOF */

reactos/ntoskrnl/ke/wait.c

@@ -680,7 +680,7 @@ DontWait:
 
     /* Release & Return */
     DPRINT("Returning, %x. Status: %d\n. We did not wait.",
-            KeGetCurrentThread(), WaitStatus);
+             KeGetCurrentThread(), WaitStatus);
     KeReleaseDispatcherDatabaseLock(CurrentThread->WaitIrql);
     return WaitStatus;
 }

reactos/ntoskrnl/ps/psmgr.c

@@ -78,7 +78,6 @@ PiInitProcessManager(VOID)
    PsInitProcessManagment();
    PsInitThreadManagment();
    PsInitIdleThread();
-   PsInitialiseW32Call();
 }
 
 VOID

reactos/ntoskrnl/ps/win32.c

@@ -212,89 +212,4 @@ NtW32Call(IN ULONG RoutineIndex,
     return(CallbackStatus);
 }
 
-#ifndef ALEX_CB_REWRITE
-NTSTATUS STDCALL
-NtCallbackReturn (PVOID		Result,
-		  ULONG		ResultLength,
-		  NTSTATUS	Status)
-{
-  PULONG OldStack;
-  PETHREAD Thread;
-  PNTSTATUS CallbackStatus;
-  PULONG CallerResultLength;
-  PVOID* CallerResult;
-  PVOID InitialStack;
-  PVOID StackBase;
-  ULONG_PTR StackLimit;
-  KIRQL oldIrql;
-  PNTW32CALL_SAVED_STATE State;
-  PKTRAP_FRAME SavedTrapFrame;
-  PVOID SavedCallbackStack;
-  PVOID SavedExceptionStack;
-
-  PAGED_CODE();
-
-  Thread = PsGetCurrentThread();
-  if (Thread->Tcb.CallbackStack == NULL)
-    {
-      return(STATUS_NO_CALLBACK_ACTIVE);
-    }
-
-  OldStack = (PULONG)Thread->Tcb.CallbackStack;
-
-  /*
-   * Get the values that NtW32Call left on the inactive stack for us.
-   */
-  State = (PNTW32CALL_SAVED_STATE)OldStack[0];
-  CallbackStatus = State->CallbackStatus;
-  CallerResultLength = State->CallerResultLength;
-  CallerResult = State->CallerResult;
-  InitialStack = State->SavedInitialStack;
-  StackBase = State->SavedStackBase;
-  StackLimit = State->SavedStackLimit;
-  SavedTrapFrame = State->SavedTrapFrame;
-  SavedCallbackStack = State->SavedCallbackStack;
-  SavedExceptionStack = State->SavedExceptionStack;
-
-  /*
-   * Copy the callback status and the callback result to NtW32Call
-   */
-  *CallbackStatus = Status;
-  if (CallerResult != NULL && CallerResultLength != NULL)
-    {
-      if (Result == NULL)
-	{
-	  *CallerResultLength = 0;
-	}
-      else
-	{
-	  *CallerResultLength = min(ResultLength, *CallerResultLength);
-	  RtlCopyMemory(*CallerResult, Result, *CallerResultLength);
-	}
-    }
-
-  /*
-   * Restore the old stack.
-   */
-  KeRaiseIrql(HIGH_LEVEL, &oldIrql);
-  if ((Thread->Tcb.NpxState & NPX_STATE_VALID) &&
-      &Thread->Tcb != KeGetCurrentPrcb()->NpxThread)
-    {
-      RtlCopyMemory((char*)InitialStack - sizeof(FX_SAVE_AREA),
-                    (char*)Thread->Tcb.InitialStack - sizeof(FX_SAVE_AREA),
-                    sizeof(FX_SAVE_AREA));
-    }
-  Thread->Tcb.InitialStack = InitialStack;
-  Thread->Tcb.StackBase = StackBase;
-  Thread->Tcb.StackLimit = StackLimit;
-  Thread->Tcb.TrapFrame = SavedTrapFrame;
-  Thread->Tcb.CallbackStack = SavedCallbackStack;
-  KeGetCurrentKPCR()->TSS->Esp0 = (ULONG)SavedExceptionStack;
-  KeStackSwitchAndRet((PVOID)(OldStack + 1));
-
-  /* Should never return. */
-  KEBUGCHECK(0);
-  return(STATUS_UNSUCCESSFUL);
-}
-#endif
 /* EOF */

reactos/subsys/win32k/ntuser/callback.c

@@ -182,7 +182,7 @@ co_IntCallWindowProc(WNDPROC Proc,
    Arguments->wParam = wParam;
    Arguments->lParam = lParam;
    Arguments->lParamBufferSize = lParamBufferSize;
-   ResultPointer = Arguments;
+   ResultPointer = NULL;
    ResultLength = ArgumentLength;
 
    UserLeaveCo();
@@ -193,6 +193,9 @@ co_IntCallWindowProc(WNDPROC Proc,
                       &ResultPointer,
                       &ResultLength);
 
+   /* Simulate old behaviour: copy into our local buffer */
+   RtlMoveMemory(Arguments, ResultPointer, ArgumentLength);
+
    UserEnterCo();
 
    if (!NT_SUCCESS(Status))
@@ -224,7 +227,7 @@ co_IntLoadSysMenuTemplate()
    PVOID ResultPointer;
    ULONG ResultLength;
 
-   ResultPointer = &Result;
+   ResultPointer = NULL;
    ResultLength = sizeof(LRESULT);
 
    UserLeaveCo();
@@ -235,6 +238,9 @@ co_IntLoadSysMenuTemplate()
                       &ResultPointer,
                       &ResultLength);
 
+   /* Simulate old behaviour: copy into our local buffer */
+   Result = *(LRESULT*)ResultPointer;
+
    UserEnterCo();
 
    if (!NT_SUCCESS(Status))
@@ -253,7 +259,7 @@ co_IntLoadDefaultCursors(VOID)
    ULONG ResultLength;
    BOOL DefaultCursor = TRUE;
 
-   ResultPointer = &Result;
+   ResultPointer = NULL;
    ResultLength = sizeof(LRESULT);
 
    UserLeaveCo();
@@ -264,6 +270,9 @@ co_IntLoadDefaultCursors(VOID)
                       &ResultPointer,
                       &ResultLength);
 
+   /* Simulate old behaviour: copy into our local buffer */
+   Result = *(LRESULT*)ResultPointer;
+
    UserEnterCo();
 
    if (!NT_SUCCESS(Status))
@@ -384,7 +393,7 @@ co_IntCallHookProc(INT HookId,
          break;
    }
 
-   ResultPointer = &Result;
+   ResultPointer = NULL;
    ResultLength = sizeof(LRESULT);
 
    UserLeaveCo();
@@ -395,6 +404,9 @@ co_IntCallHookProc(INT HookId,
                       &ResultPointer,
                       &ResultLength);
 
+   /* Simulate old behaviour: copy into our local buffer */
+   Result = *(LRESULT*)ResultPointer;
+
    UserEnterCo();
 
    IntCbFreeMemory(Argument);