[sginsberg] 68886: - Small patch to fix (delayed) loading of floating point state. Enables the special handling of fpu exceptions caused by frstor when used by the kernel, by implementing its use i...

Author: sginsberg
Date: Tue Sep  1 00:20:57 2015
New Revision: 68886

URL: http://svn.reactos.org/svn/reactos?rev=68886&view=rev
Log:
- Small patch to fix (delayed) loading of floating point state. Enables the special handling of fpu exceptions caused by frstor when used by the kernel, by implementing its use in assembly so the trap handler gets the instruction address it needs (for ALl compilers!). May or may not fix the fact that floating point state is not properly saved between threads.

See CORE-10005.

Modified:
    trunk/reactos/ntoskrnl/include/internal/i386/intrin_i.h
    trunk/reactos/ntoskrnl/include/internal/i386/ke.h
    trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S
    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c

Modified: trunk/reactos/ntoskrnl/include/internal/i386/intrin_i.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i...
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/i386/intrin_i.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/i386/intrin_i.h	[iso-8859-1] Tue Sep  1 00:20:57 2015
@@ -47,21 +47,6 @@
     else
     {
         __asm__ __volatile__ ("fnsave %0\n wait\n" : : "m"(*SaveArea));
-    }
-}
-
-FORCEINLINE
-VOID
-Ke386LoadFpuState(IN PFX_SAVE_AREA SaveArea)
-{
-    extern ULONG KeI386FxsrPresent;
-    if (KeI386FxsrPresent)
-    {
-        __asm__ __volatile__ ("fxrstor %0\n" : "=m"(SaveArea) : );
-    }
-    else
-    {
-        __asm__ __volatile__ (".globl _FrRestore\n _FrRestore: \n frstor %0\n wait\n" : "=m"(SaveArea) : );
     }
 }

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] Tue Sep  1 00:20:57 2015
@@ -541,7 +541,6 @@
 extern VOID NTAPI ExpInterlockedPopEntrySListResume(VOID);
 extern VOID __cdecl CopyParams(VOID);
 extern VOID __cdecl ReadBatch(VOID);
-extern VOID __cdecl FrRestore(VOID);
 extern CHAR KiSystemCallExitBranch[];
 extern CHAR KiSystemCallExit[];
 extern CHAR KiSystemCallExit2[];

Modified: trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/ctxswitch....
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/ctxswitch.S	[iso-8859-1] Tue Sep  1 00:20:57 2015
@@ -159,4 +159,28 @@
     popad
     ret 4

+PUBLIC _FrRestore
+PUBLIC @Ke386LoadFpuState@4
+@Ke386LoadFpuState@4:
+
+    /* Check if we have FXSR and choose which operand to use */
+    test byte ptr [_KeI386FxsrPresent], 1
+    jz _FrRestore
+
+    /* Restore all the FPU, MMX, XMM and MXCSR registers */
+    fxrstor [ecx]
+    ret
+
+    /*
+     * Just restore the basic FPU registers.
+     * This may raise an exception depending
+     * on the status word, which KiNpxHandler will
+     * need to check for and handle during delayed load
+     * to avoid raising an unhandled exception
+     * and crashing the system.
+     */
+_FrRestore:
+    frstor [ecx]
+    ret
+
 END

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] Tue Sep  1 00:20:57 2015
@@ -14,6 +14,9 @@

VOID KiFastCallEntry(VOID);
 VOID KiFastCallEntryWithSingleStep(VOID);
+
+extern PVOID FrRestore;
+VOID FASTCALL Ke386LoadFpuState(IN PFX_SAVE_AREA SaveArea);

/* GLOBALS ********************************************************************/

@@ -251,16 +254,16 @@
         SaveArea->Cr0NpxState |= CR0_TS;

/* Only valid if it happened during a restore */
-        //if ((PVOID)TrapFrame->Eip == FrRestore)
+        if ((PVOID)TrapFrame->Eip == FrRestore)
         {
             /* It did, so just skip the instruction */
-            //TrapFrame->Eip += 3; /* sizeof(FRSTOR) */
-            //KiEoiHelper(TrapFrame);
+            TrapFrame->Eip += 3; /* Size of FRSTOR instruction */
+            KiEoiHelper(TrapFrame);
         }
     }

/* User or kernel trap -- get ready to issue an exception */
-    //if (Thread->NpxState == NPX_STATE_NOT_LOADED)
+    if (Thread->NpxState == NPX_STATE_NOT_LOADED)
     {
         /* Update CR0 */
         Cr0 = __readcr0();
@@ -744,7 +747,7 @@
     KiEnterTrap(TrapFrame);

/* Try to handle NPX delay load */
-    while (TRUE)
+    for (;;)
     {
         /* Get the current thread */
         Thread = KeGetCurrentThread();
@@ -775,15 +778,14 @@
                 NpxSaveArea = KiGetThreadNpxArea(NpxThread);

/* Save FPU state */
-                DPRINT("FIXME: Save FPU state: %p\n", NpxSaveArea);
-                //Ke386SaveFpuState(NpxSaveArea);
+                Ke386SaveFpuState(NpxSaveArea);

/* Update NPX state */
                 NpxThread->NpxState = NPX_STATE_NOT_LOADED;
            }

/* Load FPU state */
-            //Ke386LoadFpuState(SaveArea);
+            Ke386LoadFpuState(SaveArea);

/* Update NPX state */
             Thread->NpxState = NPX_STATE_LOADED;
@@ -823,7 +825,7 @@
     {
         /*
          * If it's incorrectly set, then maybe the state is actually still valid
-         * but we could've lock track of that due to a BIOS call.
+         * but we could have lost track of that due to a BIOS call.
          * Make sure MP is still set, which should verify the theory.
          */
         if (Cr0 & CR0_MP)