[sginsberg] 43958: - Finish the work around for the Pentium cmpxchg8b lock errata: We detected the errata and allocated the 7 first IDT entries on a write protected page, but the final piece of the work around, detecting the write fault ti the Invalid Opcode handler, was missing. Implemented this in the page fault handler to detect and dispatch the write fault to the Invalid Opcode handler. - Fix the "fix" of un-protecting the 7 IDT entries on P5 in HAL's BIOS call code when setting the custom I

Author: sginsberg
Date: Wed Nov  4 22:57:32 2009
New Revision: 43958

URL: http://svn.reactos.org/svn/reactos?rev=43958&view=rev
Log:
- Finish the work around for the Pentium cmpxchg8b lock errata: We detected the errata and
allocated the 7 first IDT entries on a write protected page, but the final piece of the
work around, detecting the write fault ti the Invalid Opcode handler, was missing.
Implemented this in the page fault handler to detect and dispatch the write fault to the
Invalid Opcode handler.
- Fix the "fix" of un-protecting the 7 IDT entries on P5 in HAL's BIOS call
code when setting the custom Invalid Opcode handler. The IDT was unprotected but the write
protection wasn't reapplied after the BIOS call, breaking the work around.

Other:
- KdDebuggerEnabled is a BOOLEAN, so don't do a dword compare in KeUpdateSystemTime.
- Use better comment for the hack where we always allow page faults to be handled, even if
they the fault occured with interrupts disabled.

Modified:
    trunk/reactos/hal/halx86/generic/bios.c
    trunk/reactos/hal/halx86/generic/reboot.c
    trunk/reactos/ntoskrnl/ke/i386/systimer.S
    trunk/reactos/ntoskrnl/ke/i386/trap.s

Modified: trunk/reactos/hal/halx86/generic/bios.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/generic/bios.c?…
==============================================================================
--- trunk/reactos/hal/halx86/generic/bios.c [iso-8859-1] (original)
+++ trunk/reactos/hal/halx86/generic/bios.c [iso-8859-1] Wed Nov  4 22:57:32 2009
@@ -109,7 +109,11 @@
     ULONG Handler;
     PHARDWARE_PTE IdtPte;
 
-    /* On i586, the first 7 entries of IDT are write-protected, unprotect them. */ //
Nasty hto hack
+    /* 
+     * On P5, the first 7 entries of the IDT are write protected to work around
+     * the cmpxchg8b lock errata. Unprotect them here so we can set our custom
+     * invalid op-code handler.
+     */
     if (KeGetCurrentPrcb()->CpuType == 5)
     {
         IdtPte = GetPteAddress(((PKIPCR)KeGetPcr())->IDT);
@@ -160,6 +164,8 @@
 NTAPI
 HalpRestoreTrapHandlers(VOID)
 {
+    PHARDWARE_PTE IdtPte;
+
     /* We're back, restore the handlers we over-wrote */
     ((PKIPCR)KeGetPcr())->IDT[13].ExtendedOffset =
     (USHORT)((HalpGpfHandler >> 16) & 0xFFFF);
@@ -167,6 +173,16 @@
     ((PKIPCR)KeGetPcr())->IDT[6].ExtendedOffset =
         (USHORT)((HalpBopHandler >> 16) & 0xFFFF);
     ((PKIPCR)KeGetPcr())->IDT[6].Offset = (USHORT)HalpBopHandler;
+
+    /* On P5, restore the write protection for the first 7 IDT entries */
+    if (KeGetCurrentPrcb()->CpuType == 5)
+    {
+        IdtPte = GetPteAddress(((PKIPCR)KeGetPcr())->IDT);
+        IdtPte->Write = 0;
+
+        /* Flush the TLB by resetting CR3 */
+        __writecr3(__readcr3());
+    }
 }
 
 VOID
@@ -197,7 +213,7 @@
         Pte = GetPteAddress((PVOID)i);
         Pte->Valid = 0;
         Pte->Write = 0;
-        //Pte->Owner = 0; // Missing this?
+        Pte->Owner = 0;
         Pte->PageFrameNumber = 0;
     }
     

Modified: trunk/reactos/hal/halx86/generic/reboot.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/generic/reboot.…
==============================================================================
--- trunk/reactos/hal/halx86/generic/reboot.c [iso-8859-1] (original)
+++ trunk/reactos/hal/halx86/generic/reboot.c [iso-8859-1] Wed Nov  4 22:57:32 2009
@@ -49,7 +49,7 @@
     /* Enable warm reboot */
     ((PUSHORT)ZeroPageMapping)[0x239] = 0x1234;
 
-    /* Lock CMOS Access */
+    /* Lock CMOS Access (and disable interrupts) */
     HalpAcquireSystemHardwareSpinLock();
 
     /* Setup control register B */

Modified: trunk/reactos/ntoskrnl/ke/i386/systimer.S
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/systimer.…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/systimer.S [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/systimer.S [iso-8859-1] Wed Nov  4 22:57:32 2009
@@ -114,7 +114,7 @@
 
     /* Check if the debugger is enabled */
     cmp byte ptr __KdDebuggerEnabled, 0
-    je ResetDpcTime
+    jz ResetDpcTime
 
     /* Breakpoint */
     call _DbgBreakPoint@0
@@ -338,7 +338,7 @@
 
 DebugCheck:
     /* Check if the debugger is enabled */
-    cmp dword ptr __KdDebuggerEnabled, 0
+    cmp byte ptr __KdDebuggerEnabled, 0
     jnz DebuggerEnabled
 
     /* Check if this was a full tick */

Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?re…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] Wed Nov  4 22:57:32 2009
@@ -947,6 +947,7 @@
     /* Enter V86 Trap */
     V86_TRAP_PROLOG kit6_a, kit6_v
 
+VdmOpCodeFault:
     /* Not yet supported (Invalid OPCODE from V86) */
     UNHANDLED_PATH
 
@@ -957,6 +958,7 @@
     /* Enter trap */
     TRAP_PROLOG kit6_a, kit6_t
 
+DispatchLockErrata:
     /* Check if this happened in kernel mode */
     test byte ptr [ebp+KTRAP_FRAME_CS], MODE_MASK
     jz KmodeOpcode
@@ -1949,14 +1951,21 @@
 NoFixUp:
     mov edi, cr2
 
-    /* REACTOS Mm Hack of Doom */
+    /* Check if this processor has the cmpxchg8b lock errata */
+    cmp byte ptr _KiI386PentiumLockErrataPresent, 0
+    jnz HandleLockErrata
+
+NotLockErrata:
+    /* HACK: Handle page faults with interrupts disabled */
     test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_INTERRUPT_MASK
     je HandlePf
 
     /* Enable interrupts and check if we got here with interrupts disabled */
     sti
-    /* test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_INTERRUPT_MASK
-    jz IllegalState */
+#ifdef HACK_ABOVE_FIXED
+    test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_INTERRUPT_MASK
+    jz IllegalState
+#endif
 
 HandlePf:
     /* Send trap frame and check if this is kernel-mode or usermode */
@@ -2075,6 +2084,33 @@
 
     /* FIXME: NOT SUPPORTED */
     UNHANDLED_PATH
+
+HandleLockErrata:
+
+    /* Fail if this isn't a write fault */
+    test word ptr [ebp+KTRAP_FRAME_ERROR_CODE], 0x4
+    jnz NotLockErrata
+
+    /* Also make sure the page fault is for IDT entry 6 */
+    mov eax, PCR[KPCR_IDT]
+    add eax, 0x30
+    cmp eax, edi
+    jne NotLockErrata
+
+    /*
+     * This is a write fault to the Invalid Opcode handler entry.
+     * We assume this is the lock errata and not a real write fault.
+     */
+
+    /* Clear the error code */
+    and dword ptr [ebp+KTRAP_FRAME_ERROR_CODE], 0
+
+    /* Check if this happened in V86 mode */
+    test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_V86_MASK
+    jnz VdmOpCodeFault
+
+    /* Dispatch this to the invalid opcode handler */
+    jmp DispatchLockErrata
 .endfunc
 
 .func KiTrap0F