[ros-diffs] [reactos] 02/03: [NTOS:KD][KDBG] Get rid of kdmemsup.c as its functionality has been superseded by that implemented in mm/arm3/mmdbg.c.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=aff644a1a2c98593a8d9d…

commit aff644a1a2c98593a8d9d539668bdf6df98f077b
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Sun Nov 3 23:36:40 2019 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Sun Nov 3 23:46:52 2019 +0100

    [NTOS:KD][KDBG] Get rid of kdmemsup.c as its functionality has been superseded by that
implemented in mm/arm3/mmdbg.c.
    
    - Import KdpCopyMemoryChunks() from kd64/kdapi.c, and re-implement
      KdbpSafeReadMemory() and KdbpSafeWriteMemory() around it.
      Note that these functions read virtual memory and are equivalent of
      the kd64 KdpReadVirtualMemory() and KdpWriteVirtualMemory()
      respectively.
    
    - Get rid of the KdpEnableSafeMem() call in KdInitSystem().
    - Adjust kd gdbstub.c wrapper in accordance.
---
 ntoskrnl/kd/amd64/kdmemsup.c   | 217 -----------------------------------------
 ntoskrnl/kd/i386/kdmemsup.c    |  14 ++-
 ntoskrnl/kd/kdinit.c           |   3 -
 ntoskrnl/kd/wrappers/gdbstub.c |  26 ++++-
 ntoskrnl/kdbg/kdb.c            | 123 ++++++++++++++++-------
 ntoskrnl/ntos.cmake            |   4 +-
 6 files changed, 122 insertions(+), 265 deletions(-)

diff --git a/ntoskrnl/kd/amd64/kdmemsup.c b/ntoskrnl/kd/amd64/kdmemsup.c
deleted file mode 100644
index 75f19a1cbce..00000000000
--- a/ntoskrnl/kd/amd64/kdmemsup.c
+++ /dev/null
@@ -1,217 +0,0 @@
-/*
- * COPYRIGHT:       See COPYING in the top level directory
- * PROJECT:         ReactOS Kernel
- * FILE:            ntoskrnl/kd/amd64/kdmemsup.c
- * PURPOSE:         Kernel Debugger Safe Memory Support
- *
- * PROGRAMMERS:     arty
- */
-
-#include <ntoskrnl.h>
-#define NDEBUG
-#include <debug.h>
-
-#define HIGH_PHYS_MASK 0x80000000
-#define PAGE_TABLE_MASK 0x3ff
-#define BIG_PAGE_SIZE (1<<22)
-#define CR4_PAGE_SIZE_BIT 0x10
-#define PDE_PRESENT_BIT 0x01
-#define PDE_W_BIT 0x02
-#define PDE_PWT_BIT 0x08
-#define PDE_PCD_BIT 0x10
-#define PDE_ACCESSED_BIT 0x20
-#define PDE_DIRTY_BIT 0x40
-#define PDE_PS_BIT 0x80
-
-#define MI_KDBG_TMP_PAGE_1 (HYPER_SPACE + 0x400000 - PAGE_SIZE)
-#define MI_KDBG_TMP_PAGE_0 (MI_KDBG_TMP_PAGE_1 - PAGE_SIZE)
-
-/* VARIABLES ***************************************************************/
-
-static BOOLEAN KdpPhysAccess = FALSE;
-
-static
-ULONG_PTR
-KdpPhysMap(ULONG_PTR PhysAddr, LONG Len)
-{
-    MMPTE TempPte;
-    PMMPTE PointerPte;
-    ULONG_PTR VirtAddr;
-
-    TempPte.u.Long = PDE_PRESENT_BIT | PDE_W_BIT | PDE_PWT_BIT |
-                     PDE_PCD_BIT | PDE_ACCESSED_BIT | PDE_DIRTY_BIT;
-
-    if ((PhysAddr & (PAGE_SIZE - 1)) + Len > PAGE_SIZE)
-    {
-        TempPte.u.Hard.PageFrameNumber = (PhysAddr >> PAGE_SHIFT) + 1;
-        PointerPte = MiAddressToPte((PVOID)MI_KDBG_TMP_PAGE_1);
-        *PointerPte = TempPte;
-        VirtAddr = (ULONG_PTR)PointerPte << 10;
-        KeInvalidateTlbEntry((PVOID)VirtAddr);
-    }
-
-    TempPte.u.Hard.PageFrameNumber = PhysAddr >> PAGE_SHIFT;
-    PointerPte = MiAddressToPte((PVOID)MI_KDBG_TMP_PAGE_0);
-    *PointerPte = TempPte;
-    VirtAddr = (ULONG_PTR)PointerPte << 10;
-    KeInvalidateTlbEntry((PVOID)VirtAddr);
-
-    return VirtAddr + (PhysAddr & (PAGE_SIZE - 1));
-}
-
-static
-ULONGLONG
-KdpPhysRead(ULONG_PTR PhysAddr, LONG Len)
-{
-    ULONG_PTR Addr;
-    ULONGLONG Result = 0;
-
-    Addr = KdpPhysMap(PhysAddr, Len);
-
-    switch (Len)
-    {
-    case 8:
-        Result = *((PULONGLONG)Addr);
-        break;
-    case 4:
-        Result = *((PULONG)Addr);
-        break;
-    case 2:
-        Result = *((PUSHORT)Addr);
-        break;
-    case 1:
-        Result = *((PUCHAR)Addr);
-        break;
-    }
-
-    return Result;
-}
-
-static
-VOID
-KdpPhysWrite(ULONG_PTR PhysAddr, LONG Len, ULONGLONG Value)
-{
-    ULONG_PTR Addr;
-
-    Addr = KdpPhysMap(PhysAddr, Len);
-
-    switch (Len)
-    {
-    case 8:
-        *((PULONGLONG)Addr) = Value;
-        break;
-    case 4:
-        *((PULONG)Addr) = Value;
-        break;
-    case 2:
-        *((PUSHORT)Addr) = Value;
-        break;
-    case 1:
-        *((PUCHAR)Addr) = Value;
-        break;
-    }
-}
-
-BOOLEAN
-NTAPI
-KdpTranslateAddress(ULONG_PTR Addr, PULONG_PTR ResultAddr)
-{
-    ULONG_PTR CR3Value = __readcr3();
-    ULONG_PTR CR4Value = __readcr4();
-    ULONG_PTR PageDirectory = (CR3Value & ~(PAGE_SIZE-1)) +
-        ((Addr >> 22) * sizeof(ULONG));
-    ULONG_PTR PageDirectoryEntry = KdpPhysRead(PageDirectory, sizeof(ULONG));
-
-    /* Not present -> fail */
-    if (!(PageDirectoryEntry & PDE_PRESENT_BIT))
-    {
-        return FALSE;
-    }
-
-    /* Big Page? */
-    if ((PageDirectoryEntry & PDE_PS_BIT) && (CR4Value &
CR4_PAGE_SIZE_BIT))
-    {
-        *ResultAddr = (PageDirectoryEntry & ~(BIG_PAGE_SIZE-1)) +
-            (Addr & (BIG_PAGE_SIZE-1));
-        return TRUE;
-    }
-    else
-    {
-        ULONG_PTR PageTableAddr =
-            (PageDirectoryEntry & ~(PAGE_SIZE-1)) +
-            ((Addr >> PAGE_SHIFT) & PAGE_TABLE_MASK) * sizeof(ULONG);
-        ULONG_PTR PageTableEntry = KdpPhysRead(PageTableAddr, sizeof(ULONG));
-        if (PageTableEntry & PDE_PRESENT_BIT)
-        {
-            *ResultAddr = (PageTableEntry & ~(PAGE_SIZE-1)) +
-                (Addr & (PAGE_SIZE-1));
-            return TRUE;
-        }
-    }
-
-    return FALSE;
-}
-
-BOOLEAN
-NTAPI
-KdpSafeReadMemory(ULONG_PTR Addr, LONG Len, PVOID Value)
-{
-    ULONG_PTR ResultPhysAddr;
-
-    if (!KdpPhysAccess)
-    {
-        memcpy(Value, (PVOID)Addr, Len);
-        return TRUE;
-    }
-
-    memset(Value, 0, Len);
-
-    if (!KdpTranslateAddress(Addr, &ResultPhysAddr))
-        return FALSE;
-
-    switch (Len)
-    {
-    case 8:
-        *((PULONGLONG)Value) = KdpPhysRead(ResultPhysAddr, Len);
-        break;
-    case 4:
-        *((PULONG)Value) = KdpPhysRead(ResultPhysAddr, Len);
-        break;
-    case 2:
-        *((PUSHORT)Value) = KdpPhysRead(ResultPhysAddr, Len);
-        break;
-    case 1:
-        *((PUCHAR)Value) = KdpPhysRead(ResultPhysAddr, Len);
-        break;
-    }
-
-    return TRUE;
-}
-
-BOOLEAN
-NTAPI
-KdpSafeWriteMemory(ULONG_PTR Addr, LONG Len, ULONGLONG Value)
-{
-    ULONG_PTR ResultPhysAddr;
-
-    if (!KdpPhysAccess)
-    {
-        memcpy((PVOID)Addr, &Value, Len);
-        return TRUE;
-    }
-
-    if (!KdpTranslateAddress(Addr, &ResultPhysAddr))
-        return FALSE;
-
-    KdpPhysWrite(ResultPhysAddr, Len, Value);
-    return TRUE;
-}
-
-VOID
-NTAPI
-KdpEnableSafeMem(VOID)
-{
-    KdpPhysAccess = TRUE;
-}
-
-/* EOF */
diff --git a/ntoskrnl/kd/i386/kdmemsup.c b/ntoskrnl/kd/i386/kdmemsup.c
index 7616734ce53..20a5824deb2 100644
--- a/ntoskrnl/kd/i386/kdmemsup.c
+++ b/ntoskrnl/kd/i386/kdmemsup.c
@@ -1,3 +1,11 @@
+/*****************************************************************************\
+ *                                                                           *
+ *     This file is current kept ONLY for DOCUMENTATION purposes, until      *
+ *  we are sure that all the functionality (e.g. regarding the "big pages")  *
+ * are fully present in Mm and in mm/ARM3/mmdbg.c that supersedes this file. *
+ *                                                                           *
+\*****************************************************************************/
+
 /*
  * COPYRIGHT:       See COPYING in the top level directory
  * PROJECT:         ReactOS Kernel
@@ -44,14 +52,14 @@ KdpPhysMap(ULONG_PTR PhysAddr, LONG Len)
     if ((PhysAddr & (PAGE_SIZE - 1)) + Len > PAGE_SIZE)
     {
         TempPte.u.Hard.PageFrameNumber = (PhysAddr >> PAGE_SHIFT) + 1;
-        PointerPte = MiAddressToPte(MI_KDBG_TMP_PAGE_1);
+        PointerPte = MiAddressToPte((PVOID)MI_KDBG_TMP_PAGE_1);
         *PointerPte = TempPte;
         VirtAddr = (ULONG_PTR)PointerPte << 10;
         KeInvalidateTlbEntry((PVOID)VirtAddr);
     }
 
     TempPte.u.Hard.PageFrameNumber = PhysAddr >> PAGE_SHIFT;
-    PointerPte = MiAddressToPte(MI_KDBG_TMP_PAGE_0);
+    PointerPte = MiAddressToPte((PVOID)MI_KDBG_TMP_PAGE_0);
     *PointerPte = TempPte;
     VirtAddr = (ULONG_PTR)PointerPte << 10;
     KeInvalidateTlbEntry((PVOID)VirtAddr);
@@ -112,8 +120,8 @@ KdpPhysWrite(ULONG_PTR PhysAddr, LONG Len, ULONGLONG Value)
     }
 }
 
+static
 BOOLEAN
-NTAPI
 KdpTranslateAddress(ULONG_PTR Addr, PULONG_PTR ResultAddr)
 {
     ULONG_PTR CR3Value = __readcr3();
diff --git a/ntoskrnl/kd/kdinit.c b/ntoskrnl/kd/kdinit.c
index f5e1c489709..3e32b160920 100644
--- a/ntoskrnl/kd/kdinit.c
+++ b/ntoskrnl/kd/kdinit.c
@@ -287,9 +287,6 @@ KdInitSystem(ULONG BootPhase,
     }
     else /* BootPhase > 0 */
     {
-#ifdef _M_IX86
-        KdpEnableSafeMem();
-#endif
     }
 
     /* Call the Initialization Routines of the Registered Providers */
diff --git a/ntoskrnl/kd/wrappers/gdbstub.c b/ntoskrnl/kd/wrappers/gdbstub.c
index 6ca68939fc9..52497779078 100644
--- a/ntoskrnl/kd/wrappers/gdbstub.c
+++ b/ntoskrnl/kd/wrappers/gdbstub.c
@@ -246,19 +246,35 @@ static volatile BOOLEAN GspMemoryError = FALSE;
 static CHAR
 GspReadMemSafe(PCHAR Address)
 {
-    CHAR ch = 0;
-
-    if (!KdpSafeReadMemory((ULONG_PTR)Address, 1, &ch))
+    CHAR Ch = 0;
+
+#if 0
+    if (!NT_SUCCESS(KdpCopyMemoryChunks((ULONG64)(ULONG_PTR)Address, &Ch, 1,
+                                        0, MMDBG_COPY_UNSAFE, NULL)))
+#else
+    if (!NT_SUCCESS(MmDbgCopyMemory((ULONG64)(ULONG_PTR)Address, &Ch, 1,
+                                    MMDBG_COPY_UNSAFE)))
+#endif
+    {
         GspMemoryError = TRUE;
+    }
 
-    return ch;
+    return Ch;
 }
 
 static void
 GspWriteMemSafe(PCHAR Address, CHAR Ch)
 {
-    if (!KdpSafeWriteMemory((ULONG_PTR)Address, 1, Ch))
+#if 0
+    if (!NT_SUCCESS(KdpCopyMemoryChunks((ULONG64)(ULONG_PTR)Address, &Ch, 1,
+                                        0, MMDBG_COPY_UNSAFE | MMDBG_COPY_WRITE, NULL)))
+#else
+    if (!NT_SUCCESS(MmDbgCopyMemory((ULONG64)(ULONG_PTR)Address, &Ch, 1,
+                                    MMDBG_COPY_UNSAFE | MMDBG_COPY_WRITE)))
+#endif
+    {
         GspMemoryError = TRUE;
+    }
 }
 
 /* Convert the memory pointed to by Address into hex, placing result in Buffer
diff --git a/ntoskrnl/kdbg/kdb.c b/ntoskrnl/kdbg/kdb.c
index 6b9d4695581..d75ab137e64 100644
--- a/ntoskrnl/kdbg/kdb.c
+++ b/ntoskrnl/kdbg/kdb.c
@@ -1730,40 +1730,101 @@ KdbpGetCommandLineSettings(
     }
 }
 
+/*
+ * Copied from ntoskrnl/kd64/kdapi.c
+ */
 NTSTATUS
-KdbpSafeReadMemory(
-    OUT PVOID Dest,
-    IN PVOID Src,
-    IN ULONG Bytes)
+NTAPI
+KdpCopyMemoryChunks(IN ULONG64 Address,
+                    IN PVOID Buffer,
+                    IN ULONG TotalSize,
+                    IN ULONG ChunkSize,
+                    IN ULONG Flags,
+                    OUT PULONG ActualSize OPTIONAL)
 {
-    BOOLEAN Result = TRUE;
+    NTSTATUS Status;
+    ULONG RemainingLength, CopyChunk;
 
-    switch (Bytes)
+    /* Check if we didn't get a chunk size or if it is too big */
+    if (ChunkSize == 0)
     {
-        case 1:
-        case 2:
-        case 4:
-        case 8:
-            Result = KdpSafeReadMemory((ULONG_PTR)Src, Bytes, Dest);
-            break;
+        /* Default to 4 byte chunks */
+        ChunkSize = 4;
+    }
+    else if (ChunkSize > MMDBG_COPY_MAX_SIZE)
+    {
+        /* Normalize to maximum size */
+        ChunkSize = MMDBG_COPY_MAX_SIZE;
+    }
 
-        default:
+    /* Copy the whole range in aligned chunks */
+    RemainingLength = TotalSize;
+    CopyChunk = 1;
+    while (RemainingLength > 0)
+    {
+        /*
+         * Determine the best chunk size for this round.
+         * The ideal size is aligned, isn't larger than the
+         * the remaining length and respects the chunk limit.
+         */
+        while (((CopyChunk * 2) <= RemainingLength) &&
+               (CopyChunk < ChunkSize) &&
+               ((Address & ((CopyChunk * 2) - 1)) == 0))
         {
-            ULONG_PTR Start, End, Write;
+            /* Increase it */
+            CopyChunk *= 2;
+        }
 
-            for (Start = (ULONG_PTR)Src,
-                    End = Start + Bytes,
-                    Write = (ULONG_PTR)Dest;
-                 Result && (Start < End);
-                 Start++, Write++)
-                if (!KdpSafeReadMemory(Start, 1, (PVOID)Write))
-                    Result = FALSE;
+        /*
+         * The chunk size can be larger than the remaining size if this
+         * isn't the first round, so check if we need to shrink it back.
+         */
+        while (CopyChunk > RemainingLength)
+        {
+            /* Shrink it */
+            CopyChunk /= 2;
+        }
 
+        /* Do the copy */
+        Status = MmDbgCopyMemory(Address,
+                                 Buffer,
+                                 CopyChunk,
+                                 Flags);
+        if (!NT_SUCCESS(Status))
+        {
+            /* Copy failed, break out */
             break;
         }
+
+        /* Update pointers and length for the next run */
+        Address = Address + CopyChunk;
+        Buffer = (PVOID)((ULONG_PTR)Buffer + CopyChunk);
+        RemainingLength = RemainingLength - CopyChunk;
     }
 
-    return Result ? STATUS_SUCCESS : STATUS_ACCESS_VIOLATION;
+    /* We may have modified executable code, flush the instruction cache */
+    KeSweepICache((PVOID)(ULONG_PTR)Address, TotalSize);
+
+    /*
+     * Return the size we managed to copy and return
+     * success if we could copy the whole range.
+     */
+    if (ActualSize) *ActualSize = TotalSize - RemainingLength;
+    return RemainingLength == 0 ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL;
+}
+
+NTSTATUS
+KdbpSafeReadMemory(
+    OUT PVOID Dest,
+    IN PVOID Src,
+    IN ULONG Bytes)
+{
+    return KdpCopyMemoryChunks((ULONG64)(ULONG_PTR)Src,
+                               Dest,
+                               Bytes,
+                               0,
+                               MMDBG_COPY_UNSAFE,
+                               NULL);
 }
 
 NTSTATUS
@@ -1772,16 +1833,10 @@ KdbpSafeWriteMemory(
     IN PVOID Src,
     IN ULONG Bytes)
 {
-    BOOLEAN Result = TRUE;
-    ULONG_PTR Start, End, Write;
-
-    for (Start = (ULONG_PTR)Src,
-            End = Start + Bytes,
-            Write = (ULONG_PTR)Dest;
-         Result && (Start < End);
-         Start++, Write++)
-        if (!KdpSafeWriteMemory(Write, 1, *((PCHAR)Start)))
-            Result = FALSE;
-
-    return Result ? STATUS_SUCCESS : STATUS_ACCESS_VIOLATION;
+    return KdpCopyMemoryChunks((ULONG64)(ULONG_PTR)Dest,
+                               Src,
+                               Bytes,
+                               0,
+                               MMDBG_COPY_UNSAFE | MMDBG_COPY_WRITE,
+                               NULL);
 }
diff --git a/ntoskrnl/ntos.cmake b/ntoskrnl/ntos.cmake
index 38dc52aa787..9d4d6685408 100644
--- a/ntoskrnl/ntos.cmake
+++ b/ntoskrnl/ntos.cmake
@@ -369,7 +369,6 @@ if(NOT _WINKD_)
     if(ARCH STREQUAL "i386")
         list(APPEND SOURCE
             ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/i386/kdbg.c
-            ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/i386/kdmemsup.c
             ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/wrappers/gdbstub.c)
         if(KDBG)
             list(APPEND ASM_SOURCE ${REACTOS_SOURCE_DIR}/ntoskrnl/kdbg/i386/kdb_help.S)
@@ -377,9 +376,8 @@ if(NOT _WINKD_)
         endif()
     elseif(ARCH STREQUAL "amd64")
         list(APPEND SOURCE
-            ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/amd64/kd.c
             ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/i386/kdbg.c  # Use the x86 file
-            ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/amd64/kdmemsup.c)
+            ${REACTOS_SOURCE_DIR}/ntoskrnl/kd/amd64/kd.c)
         if(KDBG)
             list(APPEND ASM_SOURCE ${REACTOS_SOURCE_DIR}/ntoskrnl/kdbg/amd64/kdb_help.S)
             list(APPEND SOURCE