[sginsberg] 68907: - Implement super-simple KeSweepICache that always flushes the entire instruction cache. Use it in KD64 after modifying memory to make sure CPU gets the correct code to execute, ...

Author: sginsberg
Date: Tue Sep  1 23:45:48 2015
New Revision: 68907

URL: http://svn.reactos.org/svn/reactos?rev=68907&view=rev
Log:
- Implement super-simple KeSweepICache that always flushes the entire instruction cache.
Use it in KD64 after modifying memory to make sure CPU gets the correct code to execute,
and in NtFlushInstructionCache. May improve breakpoints somewhat.
- Move NtFlushInstructionCache from sysinfo.c to virtual.c where it fits better. Likewise,
move it from kefuncs to mmfuncs in NDK, and fix function arguments (ULONG -> SIZE_T).
- Re-enable TRAP_DEBUG, adding back critical checks in the trap code. Checks can be
improved but it is better than potentially silently messing up system state.
- Move remaining RtlPrefetchMemoryNonTemporal code into kernel. Stubbed for non-x86.
- By Hermes suggestion, override ASSERT to NT_ASSERT only for MSVC builds as that is where
the main benefit is.

Modified:
    trunk/reactos/dll/win32/kernel32/client/proc.c
    trunk/reactos/include/ndk/kefuncs.h
    trunk/reactos/include/ndk/mmfuncs.h
    trunk/reactos/lib/rtl/mem.c
    trunk/reactos/lib/rtl/powerpc/rtlmem.s
    trunk/reactos/ntoskrnl/ex/sysinfo.c
    trunk/reactos/ntoskrnl/include/internal/amd64/ke.h
    trunk/reactos/ntoskrnl/include/internal/arm/ke.h
    trunk/reactos/ntoskrnl/include/internal/i386/ke.h
    trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h
    trunk/reactos/ntoskrnl/include/internal/powerpc/ke.h
    trunk/reactos/ntoskrnl/include/ntoskrnl.h
    trunk/reactos/ntoskrnl/kd64/kdapi.c
    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
    trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
    trunk/reactos/ntoskrnl/rtl/misc.c

Modified: trunk/reactos/dll/win32/kernel32/client/proc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/…
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/proc.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/proc.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -1507,12 +1507,12 @@
 WINAPI
 FlushInstructionCache(IN HANDLE hProcess,
                       IN LPCVOID lpBaseAddress,
-                      IN SIZE_T dwSize)
+                      IN SIZE_T nSize)
 {
     NTSTATUS Status;
 
     /* Call the native function */
-    Status = NtFlushInstructionCache(hProcess, (PVOID)lpBaseAddress, dwSize);
+    Status = NtFlushInstructionCache(hProcess, (PVOID)lpBaseAddress, nSize);
     if (!NT_SUCCESS(Status))
     {
         /* Handle failure case */

Modified: trunk/reactos/include/ndk/kefuncs.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/kefuncs.h?rev=…
==============================================================================
--- trunk/reactos/include/ndk/kefuncs.h	[iso-8859-1] (original)
+++ trunk/reactos/include/ndk/kefuncs.h	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -398,15 +398,6 @@
     _In_ LARGE_INTEGER *Interval
 );
 
-NTSYSCALLAPI
-NTSTATUS
-NTAPI
-NtFlushInstructionCache(
-    _In_ HANDLE ProcessHandle,
-    _In_ PVOID BaseAddress,
-    _In_ ULONG NumberOfBytesToFlush
-);
-
 ULONG
 NTAPI
 NtGetCurrentProcessorNumber(

Modified: trunk/reactos/include/ndk/mmfuncs.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/mmfuncs.h?rev=…
==============================================================================
--- trunk/reactos/include/ndk/mmfuncs.h	[iso-8859-1] (original)
+++ trunk/reactos/include/ndk/mmfuncs.h	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -137,6 +137,15 @@
 NTSYSCALLAPI
 NTSTATUS
 NTAPI
+NtFlushInstructionCache(
+    _In_ HANDLE ProcessHandle,
+    _In_ PVOID BaseAddress,
+    _In_ SIZE_T NumberOfBytesToFlush
+);
+
+NTSYSCALLAPI
+NTSTATUS
+NTAPI
 NtFlushVirtualMemory(
     _In_ HANDLE ProcessHandle,
     _Inout_ PVOID *BaseAddress,

Modified: trunk/reactos/lib/rtl/mem.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/mem.c?rev=68907&am…
==============================================================================
--- trunk/reactos/lib/rtl/mem.c	[iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/mem.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -146,21 +146,6 @@
     memmove(Destination, Source, Length);
 }
 
-
-/*
-* @implemented
-*/
-VOID
-FASTCALL
-RtlPrefetchMemoryNonTemporal(IN PVOID Source,
-                             IN SIZE_T Length)
-{
-    /* By nature of prefetch, this is non-portable. */
-    (void)Source;
-    (void)Length;
-}
-
-
 #undef RtlZeroMemory
 /*
  * @implemented

Modified: trunk/reactos/lib/rtl/powerpc/rtlmem.s
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/powerpc/rtlmem.s?r…
==============================================================================
--- trunk/reactos/lib/rtl/powerpc/rtlmem.s	[iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/powerpc/rtlmem.s	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -100,6 +100,3 @@
         mr 5,4
         xor 4,4,4
         b memset
-
-RtlPrefetchMemoryNonTemporal:
-        blr

Modified: trunk/reactos/ntoskrnl/ex/sysinfo.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/sysinfo.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ex/sysinfo.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ex/sysinfo.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -2533,99 +2533,19 @@
     return Status;
 }
 
-NTSTATUS
-NTAPI
-NtFlushInstructionCache(
-    _In_ HANDLE ProcessHandle,
-    _In_opt_ PVOID BaseAddress,
-    _In_ ULONG FlushSize)
-{
-    KAPC_STATE ApcState;
-    PKPROCESS Process;
-    NTSTATUS Status;
-    PAGED_CODE();
-
-    /* Is a base address given? */
-    if (BaseAddress != NULL)
-    {
-        /* If the requested size is 0, there is nothing to do */
-        if (FlushSize == 0)
-        {
-            return STATUS_SUCCESS;
-        }
-
-        /* Is this a user mode call? */
-        if (KeGetPreviousMode() != KernelMode)
-        {
-            /* Make sure the base address is in user space */
-            if (BaseAddress > MmHighestUserAddress)
-            {
-                DPRINT1("Invalid BaseAddress 0x%p\n", BaseAddress);
-                return STATUS_ACCESS_VIOLATION;
-            }
-        }
-    }
-
-    /* Is another process requested? */
-    if (ProcessHandle != NtCurrentProcess())
-    {
-        /* Reference the process */
-        Status = ObReferenceObjectByHandle(ProcessHandle,
-                                           PROCESS_VM_WRITE,
-                                           PsProcessType,
-                                           KeGetPreviousMode(),
-                                           (PVOID*)&Process,
-                                           NULL);
-        if (!NT_SUCCESS(Status))
-        {
-            DPRINT1("Failed to reference the process %p\n", ProcessHandle);
-            return Status;
-        }
-
-        /* Attach to the process */
-        KeStackAttachProcess(Process, &ApcState);
-    }
-
-    /* FIXME: don't flush everything if a range is requested */
-#if defined(_M_IX86) || defined(_M_AMD64)
-    __wbinvd();
-#elif defined(_M_PPC)
-    __asm__ __volatile__("tlbsync");
-#elif defined(_M_MIPS)
-    DPRINT1("NtFlushInstructionCache() is not implemented\n");
-    DbgBreakPoint();
-#elif defined(_M_ARM)
-    _MoveToCoprocessor(0, CP15_ICIALLU);
-#else
-#error Unknown architecture
-#endif
-
-    /* Check if we attached */
-    if (ProcessHandle != NtCurrentProcess())
-    {
-        /* Detach from the process */
-        KeUnstackDetachProcess(&ApcState);
-        ObDereferenceObject(Process);
-    }
-
-    return STATUS_SUCCESS;
-}
-
 ULONG
 NTAPI
 NtGetCurrentProcessorNumber(VOID)
 {
-    /* Just return the CPU */
+    /* Just use Ke */
     return KeGetCurrentProcessorNumber();
 }
 
-/*
- * @implemented
- */
 #undef ExGetPreviousMode
 KPROCESSOR_MODE
 NTAPI
-ExGetPreviousMode (VOID)
-{
+ExGetPreviousMode(VOID)
+{
+    /* Just use Ke */
     return KeGetPreviousMode();
 }

Modified: trunk/reactos/ntoskrnl/include/internal/amd64/ke.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/amd64/ke.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/amd64/ke.h	[iso-8859-1] Tue Sep  1 23:45:48
2015
@@ -186,6 +186,19 @@
 {
     /* Flush the TLB by resetting CR3 */
     __writecr3(__readcr3());
+}
+
+FORCEINLINE
+VOID
+KeSweepICache(IN PVOID BaseAddress,
+              IN SIZE_T FlushSize)
+{
+    //
+    // Always sweep the whole cache
+    //
+    UNREFERENCED_PARAMETER(BaseAddress);
+    UNREFERENCED_PARAMETER(FlushSize);
+    __wbinvd();
 }
 
 FORCEINLINE

Modified: trunk/reactos/ntoskrnl/include/internal/arm/ke.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/arm/ke.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/arm/ke.h	[iso-8859-1] Tue Sep  1 23:45:48
2015
@@ -118,6 +118,19 @@
 
 FORCEINLINE
 VOID
+KeSweepICache(IN PVOID BaseAddress,
+              IN SIZE_T FlushSize)
+{
+    //
+    // Always sweep the whole cache
+    //
+    UNREFERENCED_PARAMETER(BaseAddress);
+    UNREFERENCED_PARAMETER(FlushSize);
+    _MoveToCoprocessor(0, CP15_ICIALLU);
+}
+
+FORCEINLINE
+VOID
 KiRundownThread(IN PKTHREAD Thread)
 {
     /* FIXME */

Modified: trunk/reactos/ntoskrnl/include/internal/i386/ke.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/i386/ke.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/i386/ke.h	[iso-8859-1] Tue Sep  1 23:45:48
2015
@@ -264,6 +264,19 @@
 {
     /* Flush the TLB by resetting CR3 */
     __writecr3(__readcr3());
+}
+
+FORCEINLINE
+VOID
+KeSweepICache(IN PVOID BaseAddress,
+              IN SIZE_T FlushSize)
+{
+    //
+    // Always sweep the whole cache
+    //
+    UNREFERENCED_PARAMETER(BaseAddress);
+    UNREFERENCED_PARAMETER(FlushSize);
+    __wbinvd();
 }
 
 FORCEINLINE

Modified: trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- 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] Tue Sep  1 23:45:48
2015
@@ -7,8 +7,6 @@
  */
 
 #pragma once
-
-#define TRAP_DEBUG 0
 
 #define UNREACHABLE __assume(0)
 
@@ -72,9 +70,9 @@
     DbgPrint("V86Gs: %x\n", TrapFrame->V86Gs);
 }
 
-#if TRAP_DEBUG
-VOID
-FORCEINLINE
+#if DBG
+FORCEINLINE
+VOID
 KiFillTrapFrameDebug(IN PKTRAP_FRAME TrapFrame)
 {
     /* Set the debug information */

Modified: trunk/reactos/ntoskrnl/include/internal/powerpc/ke.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/powerpc/ke.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/powerpc/ke.h	[iso-8859-1] Tue Sep  1 23:45:48
2015
@@ -90,6 +90,19 @@
 }
 
 FORCEINLINE
+VOID
+KeSweepICache(IN PVOID BaseAddress,
+              IN SIZE_T FlushSize)
+{
+    //
+    // Always sweep the whole cache
+    //
+    UNREFERENCED_PARAMETER(BaseAddress);
+    UNREFERENCED_PARAMETER(FlushSize);
+    __asm__ __volatile__("tlbsync");
+}
+
+FORCEINLINE
 PRKTHREAD
 KeGetCurrentThread(VOID)
 {

Modified: trunk/reactos/ntoskrnl/include/ntoskrnl.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/ntoskrnl.…
==============================================================================
--- trunk/reactos/ntoskrnl/include/ntoskrnl.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/ntoskrnl.h	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -98,8 +98,10 @@
 //
 // NT_ASSERT Best Assert
 //
+#if defined(_MSC_VER)
 #undef ASSERT
 #define ASSERT NT_ASSERT
+#endif
 
 /* Internal Headers */
 #include "internal/ntoskrnl.h"

Modified: trunk/reactos/ntoskrnl/kd64/kdapi.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kd64/kdapi.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -83,6 +83,11 @@
         Buffer = (PVOID)((ULONG_PTR)Buffer + CopyChunk);
         RemainingLength = RemainingLength - CopyChunk;
     }
+
+    /*
+     * We may have modified executable code, flush the instruction cache
+     */
+     KeSweepICache((PVOID)Address, TotalSize);
 
     /*
      * Return the size we managed to copy

Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -56,7 +56,7 @@
 PKDBG_PRESERVICEHOOK KeWin32PreServiceHook = NULL;
 PKDBG_POSTSERVICEHOOK KeWin32PostServiceHook = NULL;
 #endif
-#if TRAP_DEBUG
+#if DBG
 BOOLEAN StopChecking = FALSE;
 #endif
 

Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -2912,6 +2912,73 @@
     // Return status
     //
     return Status;
+}
+
+NTSTATUS
+NTAPI
+NtFlushInstructionCache(_In_ HANDLE ProcessHandle,
+                        _In_opt_ PVOID BaseAddress,
+                        _In_ SIZE_T FlushSize)
+{
+    KAPC_STATE ApcState;
+    PKPROCESS Process;
+    NTSTATUS Status;
+    PAGED_CODE();
+
+    /* Is a base address given? */
+    if (BaseAddress != NULL)
+    {
+        /* If the requested size is 0, there is nothing to do */
+        if (FlushSize == 0)
+        {
+            return STATUS_SUCCESS;
+        }
+
+        /* Is this a user mode call? */
+        if (ExGetPreviousMode() != KernelMode)
+        {
+            /* Make sure the base address is in user space */
+            if (BaseAddress > MmHighestUserAddress)
+            {
+                DPRINT1("Invalid BaseAddress 0x%p\n", BaseAddress);
+                return STATUS_ACCESS_VIOLATION;
+            }
+        }
+    }
+
+    /* Is another process requested? */
+    if (ProcessHandle != NtCurrentProcess())
+    {
+        /* Reference the process */
+        Status = ObReferenceObjectByHandle(ProcessHandle,
+                                           PROCESS_VM_WRITE,
+                                           PsProcessType,
+                                           ExGetPreviousMode(),
+                                           (PVOID*)&Process,
+                                           NULL);
+        if (!NT_SUCCESS(Status))
+        {
+            DPRINT1("Failed to reference the process %p\n", ProcessHandle);
+            return Status;
+        }
+
+        /* Attach to the process */
+        KeStackAttachProcess(Process, &ApcState);
+    }
+
+    /* Forward to Ke */
+    KeSweepICache(BaseAddress, FlushSize);
+
+    /* Check if we attached */
+    if (ProcessHandle != NtCurrentProcess())
+    {
+        /* Detach from the process and dereference it */
+        KeUnstackDetachProcess(&ApcState);
+        ObDereferenceObject(Process);
+    }
+
+    /* All done, return to caller */
+    return STATUS_SUCCESS;
 }
 
 NTSTATUS

Modified: trunk/reactos/ntoskrnl/rtl/misc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/rtl/misc.c?rev=68…
==============================================================================
--- trunk/reactos/ntoskrnl/rtl/misc.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/rtl/misc.c	[iso-8859-1] Tue Sep  1 23:45:48 2015
@@ -62,4 +62,21 @@
     return STATUS_SUCCESS;
 }
 
+#if !defined(_M_IX86)
+//
+// Stub for architectures which don't have this implemented
+//
+VOID
+FASTCALL
+RtlPrefetchMemoryNonTemporal(IN PVOID Source,
+                             IN SIZE_T Length)
+{
+    //
+    // Do nothing
+    //
+    UNREFERENCED_PARAMETER(Source);
+    UNREFERENCED_PARAMETER(Length);
+}
+#endif
+
 /* EOF */